diff --git a/crates/openloops-desktop/src/app_model.rs b/crates/openloops-desktop/src/app_model.rs index 8df2ca4..3d3689f 100644 --- a/crates/openloops-desktop/src/app_model.rs +++ b/crates/openloops-desktop/src/app_model.rs @@ -6,13 +6,14 @@ use std::sync::{Arc, atomic::Ordering}; use std::time::Instant; use crate::claim_view::LoopItem; -use crate::review_model::{CardContext, ReviewState}; +use crate::review_model::{CardContext, ConversationKey, ReviewState}; use crate::settings::{ ExtractionBackend, OllamaPlan, Provider, Settings, SettingsError, SettingsStore, max_parallel, production_store, }; use openloops_graph::live::{ - ConnectionConfig, ConnectionError, ConnectionReport, clear_session, + ConnectionConfig, ConnectionError, ConnectionReport, MailProvider, clear_all_sessions, + registration, review::{LoadProgress, MailCache}, }; use openloops_inference::decision::DecisionCheckReport; @@ -22,7 +23,7 @@ use openloops_inference::provider::ProviderError; use zeroize::Zeroizing; pub(crate) enum Outcome { - Microsoft(Result), + Connection(MailProvider, Result), Models(Result, ProviderError>), ZdrModels(Result, ProviderError>), Generation(Result<(), ProviderError>), @@ -38,17 +39,17 @@ pub(crate) enum Outcome { RetryMail { sources: Result, ConnectionError>, source_keys: BTreeSet, - conversations: BTreeSet, + conversations: BTreeSet, attempted: usize, }, RetryScan { result: Result, - conversations: BTreeSet, + conversations: BTreeSet, attempted: usize, }, CheckScan { result: Result, - conversations: BTreeSet, + conversations: BTreeSet, new_messages: usize, }, Reminder([u8; 32], openloops_graph::live::reminders::ReminderOutcome), @@ -68,7 +69,7 @@ pub(crate) enum Outcome { #[derive(Clone, Copy, PartialEq, Eq)] pub(crate) enum Service { - Microsoft, + Mailbox, Model, Review, } @@ -98,6 +99,9 @@ pub(crate) enum ExportArmed { /// Toolkit-free setup/connection/review model used by the native UI adapter. pub struct AppModel { pub client_id: String, + pub google_client_id: String, + pub google_client_secret: Zeroizing, + pub mail_providers: BTreeSet, pub groups: String, pub shared: String, pub key: Zeroizing, @@ -123,6 +127,7 @@ pub struct AppModel { /// any further folder edit or by the write attempt itself. pub(crate) training_export_armed: ExportArmed, pub(crate) microsoft: Status, + pub(crate) google: Status, pub(crate) model_status: Status, pub(crate) decision_status: Status, pub(crate) pending: Option>, @@ -169,6 +174,9 @@ impl AppModel { pub fn with_store(store: Result>, SettingsError>) -> Self { let mut app = Self { client_id: String::new(), + google_client_id: String::new(), + google_client_secret: Zeroizing::new(String::new()), + mail_providers: BTreeSet::from([MailProvider::Microsoft]), groups: String::new(), shared: String::new(), key: Zeroizing::new(String::new()), @@ -186,10 +194,11 @@ impl AppModel { training_export_status: Status::default(), training_export_armed: ExportArmed::No, microsoft: Status::default(), + google: Status::default(), model_status: Status::default(), decision_status: Status::default(), pending: None, - pending_service: Service::Microsoft, + pending_service: Service::Mailbox, progress: "", started: Instant::now(), store: None, @@ -231,6 +240,9 @@ impl AppModel { fn apply_settings(&mut self, settings: Settings) { self.clear_mail_cache(); self.client_id = settings.client_id; + self.google_client_id = settings.google_client_id; + self.google_client_secret = settings.google_client_secret; + self.mail_providers = settings.mail_providers; self.groups = settings.groups; self.shared = settings.shared; self.key = settings.key; @@ -257,6 +269,7 @@ impl AppModel { }; self.trim_keys(); self.microsoft = Status::default(); + self.google = Status::default(); self.model_status = Status::default(); self.decision_status = Status::default(); self.pending_save = false; @@ -308,6 +321,9 @@ impl AppModel { }; let settings = Settings { client_id: self.client_id.clone(), + google_client_id: self.google_client_id.clone(), + google_client_secret: self.google_client_secret.clone(), + mail_providers: self.mail_providers.clone(), groups: self.groups.clone(), shared: self.shared.clone(), key: self.key.clone(), @@ -346,7 +362,7 @@ impl AppModel { pub fn forget_settings(&mut self) { self.clear_mail_cache(); - clear_session(); + clear_all_sessions(); let Some(store) = &self.store else { return; }; @@ -403,7 +419,7 @@ impl AppModel { succeeded: false, }; match self.pending_service { - Service::Microsoft => self.microsoft = status, + Service::Mailbox => self.microsoft = status, Service::Model => self.model_status = status, Service::Review => { self.load_progress = None; @@ -482,8 +498,8 @@ impl AppModel { succeeded: true, }; } - Outcome::Microsoft(result) => { - self.microsoft = microsoft_status(result); + Outcome::Connection(provider, result) => { + *self.connection_status_mut(provider) = microsoft_status(result); } Outcome::Mail(Ok(sources)) => { self.load_progress = None; @@ -733,7 +749,7 @@ impl AppModel { use openloops_graph::live::reminders::ReminderOutcome; let mut record = self.review.decisions.get(&key); let (text, outcome_succeeded)=match outcome { - ReminderOutcome::Created{list_id,task_id}=>{record.reminder=Reminder::Created{list_id,task_id}; ("Reminder created in your Microsoft To Do Tasks list.".to_owned(), true)}, + ReminderOutcome::Created{list_id,task_id}=>{record.reminder=Reminder::Created{provider: MailProvider::Microsoft,list_id,task_id}; ("Reminder created in your Microsoft To Do Tasks list.".to_owned(), true)}, ReminderOutcome::NotCreated(reason)=>{record.reminder=Reminder::None;(format!("No reminder was created: {reason}"), false)}, ReminderOutcome::Uncertain=>("Microsoft did not confirm the write. Check To Do before trying again; OpenLoops will not automatically retry.".to_owned(), false), }; @@ -797,11 +813,20 @@ impl AppModel { continue; } let mut record = self.review.decisions.get(&key); - let Reminder::Created { list_id, task_id } = record.reminder.clone() else { + let Reminder::Created { + provider, + list_id, + task_id, + } = record.reminder.clone() + else { continue; }; record.decision = Decision::Done; - record.reminder = Reminder::Completed { list_id, task_id }; + record.reminder = Reminder::Completed { + provider, + list_id, + task_id, + }; record.updated = crate::loop_state::now(); if self.review.decisions.update(record).is_ok() { completed += 1; @@ -899,11 +924,45 @@ impl AppModel { /// the model owning any UI-nav state. #[must_use] pub fn ready_for_review(&self) -> bool { - !self.client_id.is_empty() + self.effective_microsoft_client_id().is_some() && !self.active_key().is_empty() && !self.selected_model().is_empty() } + #[must_use] + pub fn effective_microsoft_client_id(&self) -> Option { + registration::effective_client_id( + &self.client_id, + registration::microsoft().map(|value| value.client_id), + ) + } + + #[must_use] + pub fn shared_registration_active(&self) -> bool { + self.client_id.trim().is_empty() && registration::microsoft().is_some() + } + + #[must_use] + pub fn admin_consent_url(&self) -> Option { + self.effective_microsoft_client_id() + .map(|client_id| registration::microsoft_admin_consent_url(&client_id)) + } + + #[must_use] + pub(crate) const fn connection_status(&self, provider: MailProvider) -> &Status { + match provider { + MailProvider::Microsoft => &self.microsoft, + MailProvider::Google => &self.google, + } + } + + fn connection_status_mut(&mut self, provider: MailProvider) -> &mut Status { + match provider { + MailProvider::Microsoft => &mut self.microsoft, + MailProvider::Google => &mut self.google, + } + } + pub(crate) fn start_scan(&mut self, on_done: impl FnOnce() + Send + 'static) { let inputs = self.scan_job_inputs(); inputs @@ -961,7 +1020,10 @@ impl AppModel { if checks.is_empty() { return; } - let Ok(config) = ConnectionConfig::new(self.client_id.trim(), None) else { + let Some(client_id) = self.effective_microsoft_client_id() else { + return; + }; + let Ok(config) = ConnectionConfig::new(&client_id, None) else { return; }; self.start( @@ -987,7 +1049,7 @@ impl AppModel { pub(crate) fn start_retry_scan( &mut self, - conversations: BTreeSet, + conversations: BTreeSet, attempted: usize, on_done: impl FnOnce() + Send + 'static, ) { @@ -1036,7 +1098,7 @@ impl AppModel { pub(crate) fn start_check_scan( &mut self, - conversations: BTreeSet, + conversations: BTreeSet, new_messages: BTreeSet, prior_items: Vec, on_done: impl FnOnce() + Send + 'static, @@ -1151,30 +1213,21 @@ pub(crate) fn microsoft_status(result: Result } } -/// Whether `url` is an Outlook web link `OpenLoops` is willing to draw a -/// hyperlink to: exactly one of the accepted host prefixes, requiring the -/// trailing slash (so a bare host with nothing after it does not match) and -/// `https`. Guards against a host-spoofing attempt such as -/// `https://example.invalid/outlook.office.com/`, where the accepted text -/// appears but not as the scheme+host prefix. +/// Whether `url` is a message link from any supported mail provider. +/// Keep this UI gate aligned with `provider.rs::is_trusted_message_link`. #[must_use] -pub fn is_outlook_link(url: &str) -> bool { - [ - "https://outlook.office.com/", - "https://outlook.office365.com/", - "https://outlook.live.com/", - "https://outlook.office365.us/", - ] - .iter() - .any(|prefix| url.starts_with(prefix)) +pub fn is_trusted_message_link(url: &str) -> bool { + MailProvider::ALL + .iter() + .any(|provider| provider.is_trusted_message_link(url)) } -/// How the Microsoft account is shown in the title bar. +/// How connected mail providers are shown in the title bar. /// /// Owner feedback item 3: the Graph layer exposes no display name yet, so -/// there is no avatar and no name to show -- only a connection summary -/// ("Signed in · Microsoft 365") once the Microsoft connection has succeeded -/// or review messages have already loaded, and nothing at all otherwise +/// there is no avatar or account name to show -- only a stable-order, +/// per-provider connection summary once each provider's connection has +/// succeeded or its review messages have already loaded, and nothing otherwise /// (never a misleading "Not signed in" while mail is actually being /// scanned). [`AccountDisplay::connected`] builds that display. /// [`AccountDisplay::signed_in`] is kept for when the Graph layer exposes a @@ -1197,15 +1250,23 @@ impl Default for AccountDisplay { } impl AccountDisplay { - /// Builds the title bar's connection-summary display: no name, no - /// initials (so no avatar renders), and the summary line shown only - /// when `connected`. + /// Builds a stable-order summary for exactly the connected providers. #[must_use] - pub fn connected(connected: bool) -> Self { + pub fn connected(providers: &[MailProvider]) -> Self { + let services = MailProvider::ALL + .iter() + .filter(|provider| providers.contains(provider)) + .map(|provider| provider.service_name()) + .collect::>() + .join(" + "); Self { - name: String::new(), + name: if services.is_empty() { + String::new() + } else { + format!("Signed in · {services}") + }, initials: String::new(), - signed_in: connected, + signed_in: !providers.is_empty(), } } @@ -1277,6 +1338,7 @@ mod tests { fn check_source(messages: Vec) -> SourceReview { SourceReview { + provider: openloops_graph::live::MailProvider::Microsoft, label: "Inbox".into(), messages, errors: vec![], @@ -1470,6 +1532,21 @@ mod tests { assert!(reopened.use_decision_model); } + #[test] + fn apply_settings_round_trips_google_mail_settings() { + let memory = MemoryStore::default(); + let mut app = AppModel::with_store(Ok(Some(Box::new(memory.clone())))); + app.google_client_id = "123-fixture.apps.googleusercontent.com".into(); + app.google_client_secret = Zeroizing::new("fixture-secret".into()); + app.mail_providers = BTreeSet::from([MailProvider::Microsoft, MailProvider::Google]); + app.save_settings(); + + let reopened = AppModel::with_store(Ok(Some(Box::new(memory)))); + assert_eq!(reopened.google_client_id, app.google_client_id); + assert_eq!(reopened.google_client_secret, app.google_client_secret); + assert_eq!(reopened.mail_providers, app.mail_providers); + } + #[test] fn keys_are_trimmed_once_so_testing_and_scanning_use_the_same_value() { let mut app = AppModel::new(); @@ -1523,7 +1600,7 @@ mod tests { let mut app = AppModel::new(); let (sender, receiver) = mpsc::channel(); app.pending = Some(receiver); - app.pending_service = Service::Microsoft; + app.pending_service = Service::Mailbox; drop(sender); app.poll(|| {}); assert!(app.pending.is_none()); @@ -1559,6 +1636,54 @@ mod tests { assert!(app.pending.is_none()); } + #[test] + fn connection_outcome_updates_only_the_named_provider_status() { + let mut app = AppModel::with_store(Ok(None)); + let (sender, receiver) = mpsc::channel(); + app.pending = Some(receiver); + sender + .send(Outcome::Connection( + MailProvider::Google, + Err(ConnectionError::ProviderUnavailable), + )) + .unwrap(); + + assert!(app.poll(|| {})); + assert!(!app.connection_status(MailProvider::Google).lines.is_empty()); + assert!( + app.connection_status(MailProvider::Microsoft) + .lines + .is_empty() + ); + } + + #[test] + fn microsoft_registration_helpers_prefer_the_typed_id() { + let mut app = AppModel::with_store(Ok(None)); + app.client_id = "00000000-0000-4000-8000-000000000000".into(); + assert_eq!( + app.effective_microsoft_client_id().as_deref(), + Some("00000000-0000-4000-8000-000000000000") + ); + assert!(!app.shared_registration_active()); + assert!( + app.admin_consent_url() + .is_some_and(|url| url.contains("client_id=00000000-0000-4000-8000-000000000000")) + ); + } + + #[test] + fn empty_byo_without_a_shipped_registration_has_no_microsoft_registration() { + if registration::microsoft().is_some() { + return; + } + let app = AppModel::with_store(Ok(None)); + assert!(app.client_id.is_empty()); + assert!(app.effective_microsoft_client_id().is_none()); + assert!(app.admin_consent_url().is_none()); + assert!(!app.shared_registration_active()); + } + #[test] fn mail_outcome_populates_the_session_cache_and_clears_load_progress() { use openloops_graph::live::review::{LoadProgress, MailItem, SourceReview}; @@ -1566,6 +1691,7 @@ mod tests { let mut app = AppModel::with_store(Ok(None)); app.load_progress = Some(Arc::new(LoadProgress::default())); let sources = vec![SourceReview { + provider: openloops_graph::live::MailProvider::Microsoft, label: "Personal mailbox / Inbox".into(), messages: vec![ MailItem { @@ -1700,7 +1826,7 @@ mod tests { sender .send(Outcome::CheckScan { result: Ok(check_scan_result(vec![item])), - conversations: BTreeSet::from(["synthetic-thread-3".into()]), + conversations: BTreeSet::from([("synthetic".into(), "synthetic-thread-3".into())]), new_messages: 2, }) .unwrap(); @@ -1753,6 +1879,7 @@ mod tests { app.review.action_status = "Synthetic decision status".into(); app.review.action_status_succeeded = false; app.review.failed_conversations_detail = vec![crate::review_model::ConversationFailure { + account: "synthetic-account".into(), conversation: "synthetic-timeout".into(), subject_short: "Synthetic subject".into(), reason: crate::review_model::FailureReason::Timeout, @@ -1786,31 +1913,45 @@ mod tests { } #[test] - fn is_outlook_link_accepts_only_allowlisted_https_hosts_with_paths() { - assert!(is_outlook_link("https://outlook.office.com/mail/deeplink")); - assert!(is_outlook_link( + fn trusted_message_links_accept_only_allowlisted_https_hosts_with_paths() { + assert!(is_trusted_message_link( + "https://outlook.office.com/mail/deeplink" + )); + assert!(is_trusted_message_link( "https://outlook.office365.com/mail/deeplink" )); - assert!(is_outlook_link("https://outlook.live.com/mail/deeplink")); - assert!(is_outlook_link( + assert!(is_trusted_message_link( + "https://outlook.live.com/mail/deeplink" + )); + assert!(is_trusted_message_link( "https://outlook.office365.us/mail/deeplink" )); - assert!(!is_outlook_link("https://outlook.office.com")); - assert!(!is_outlook_link("https://outlook.live.com")); - assert!(!is_outlook_link( + assert!(!is_trusted_message_link("https://outlook.office.com")); + assert!(!is_trusted_message_link("https://outlook.live.com")); + assert!(!is_trusted_message_link( "https://example.invalid/outlook.office.com/" )); - assert!(!is_outlook_link( + assert!(!is_trusted_message_link( "https://outlook.office.com.evil.invalid/mail/deeplink" )); - assert!(!is_outlook_link("http://outlook.office.com/mail/deeplink")); - assert!(!is_outlook_link("http://outlook.live.com/mail/deeplink")); - assert!(!is_outlook_link( + assert!(!is_trusted_message_link( + "http://outlook.office.com/mail/deeplink" + )); + assert!(!is_trusted_message_link( + "http://outlook.live.com/mail/deeplink" + )); + assert!(!is_trusted_message_link( "https://outlook.office.com:443/mail/deeplink" )); - assert!(!is_outlook_link( + assert!(!is_trusted_message_link( "https://user@outlook.office.com/mail/deeplink" )); + assert!(is_trusted_message_link( + "https://mail.google.com/mail/u/0/#inbox/synthetic" + )); + assert!(!is_trusted_message_link( + "https://evil.example/mail.google.com/" + )); } #[test] @@ -1822,13 +1963,22 @@ mod tests { } #[test] - fn account_display_connected_has_no_name_or_initials() { - let connected = AccountDisplay::connected(true); - assert_eq!(connected.name, ""); - assert_eq!(connected.initials, ""); - assert!(connected.signed_in); + fn account_display_lists_connected_providers_in_stable_order() { + for (providers, expected) in [ + (vec![MailProvider::Microsoft], "Signed in · Microsoft 365"), + (vec![MailProvider::Google], "Signed in · Google"), + ( + vec![MailProvider::Google, MailProvider::Microsoft], + "Signed in · Microsoft 365 + Google", + ), + ] { + let connected = AccountDisplay::connected(&providers); + assert_eq!(connected.name, expected); + assert_eq!(connected.initials, ""); + assert!(connected.signed_in); + } - let not_connected = AccountDisplay::connected(false); + let not_connected = AccountDisplay::connected(&[]); assert_eq!(not_connected.name, ""); assert_eq!(not_connected.initials, ""); assert!(!not_connected.signed_in); @@ -1926,6 +2076,7 @@ mod tests { key, decision, reminder: Reminder::Created { + provider: MailProvider::Microsoft, list_id: "list".into(), task_id: "task".into(), }, @@ -1996,15 +2147,15 @@ mod tests { ), ( ReminderFailure::Rejected(ConnectionError::Transport), - "No reminder was created: Microsoft could not be reached over a secure connection.", + "No reminder was created: The mail service could not be reached over a secure connection.", ), ( ReminderFailure::Rejected(ConnectionError::AccessDenied), - "No reminder was created: Microsoft refused the To Do write. The app registration needs the delegated Tasks.ReadWrite permission and the signed-in account must consent to it. Microsoft Graph returned HTTP 403. Check consent and this signed-in account's access to the selected mailbox or group; an administrator role alone does not grant content access.", + "No reminder was created: Microsoft refused the To Do write. The app registration needs the delegated Tasks.ReadWrite permission and the signed-in account must consent to it. The mail service returned HTTP 403. Check consent and this signed-in account's access to the selected mailbox or group; an administrator role alone does not grant content access.", ), ( ReminderFailure::Rejected(ConnectionError::Unauthorized), - "No reminder was created: Microsoft refused the To Do write. The app registration needs the delegated Tasks.ReadWrite permission and the signed-in account must consent to it. Microsoft Graph returned HTTP 401. Sign in again; if it persists, check the organization's access policies.", + "No reminder was created: Microsoft refused the To Do write. The app registration needs the delegated Tasks.ReadWrite permission and the signed-in account must consent to it. The mail service returned HTTP 401. Sign in again; if it persists, check the organization's access policies.", ), ]; @@ -2041,6 +2192,7 @@ mod tests { key, decision: Decision::Watching, reminder: Reminder::Created { + provider: MailProvider::Microsoft, list_id: "list-2".into(), task_id: "task-2".into(), }, diff --git a/crates/openloops-desktop/src/loop_state.rs b/crates/openloops-desktop/src/loop_state.rs index c392ab5..a2e0d7a 100644 --- a/crates/openloops-desktop/src/loop_state.rs +++ b/crates/openloops-desktop/src/loop_state.rs @@ -1,5 +1,6 @@ //! Bounded native-preview decision projection. Never stores readable source text. use hmac::{Hmac, KeyInit, Mac}; +use openloops_graph::live::MailProvider; use sha2::Sha256; use zeroize::Zeroizing; @@ -58,6 +59,7 @@ pub enum Reminder { /// Each id is bounded to `MAX_REMINDER_ID_LEN` bytes for storage; see /// that constant for why. Created { + provider: MailProvider, list_id: String, task_id: String, }, @@ -70,6 +72,7 @@ pub enum Reminder { /// `status_pill_hint`, since the decision alone (`Done` either way) /// can't say which. Completed { + provider: MailProvider, list_id: String, task_id: String, }, @@ -432,12 +435,31 @@ fn encode(secret: &[u8; 32], records: &[Record]) -> Result>, ( bytes.push(match r.reminder { Reminder::None => 0, Reminder::Attempted => 1, - Reminder::Created { .. } => 2, - Reminder::Completed { .. } => 3, + Reminder::Created { + provider: MailProvider::Microsoft, + .. + } => 2, + Reminder::Completed { + provider: MailProvider::Microsoft, + .. + } => 3, + Reminder::Created { + provider: MailProvider::Google, + .. + } => 4, + Reminder::Completed { + provider: MailProvider::Google, + .. + } => 5, }); bytes.extend_from_slice(&r.updated.to_le_bytes()); match &r.reminder { - Reminder::Created { list_id, task_id } | Reminder::Completed { list_id, task_id } => { + Reminder::Created { + list_id, task_id, .. + } + | Reminder::Completed { + list_id, task_id, .. + } => { push_id(&mut bytes, list_id)?; push_id(&mut bytes, task_id)?; } @@ -479,14 +501,27 @@ fn decode(bytes: &[u8]) -> Result<([u8; 32], Vec), ()> { let reminder = match reminder_tag { 0 => Reminder::None, 1 => Reminder::Attempted, - 2 | 3 => { + 2..=5 => { let (list_id, remaining) = read_id(rest)?; let (task_id, remaining) = read_id(remaining)?; rest = remaining; - if reminder_tag == 2 { - Reminder::Created { list_id, task_id } + let provider = if reminder_tag <= 3 { + MailProvider::Microsoft + } else { + MailProvider::Google + }; + if reminder_tag == 2 || reminder_tag == 4 { + Reminder::Created { + provider, + list_id, + task_id, + } } else { - Reminder::Completed { list_id, task_id } + Reminder::Completed { + provider, + list_id, + task_id, + } } } _ => return Err(()), @@ -515,6 +550,7 @@ mod tests { Reminder::None, Reminder::Attempted, Reminder::Created { + provider: MailProvider::Microsoft, list_id: "list".into(), task_id: "task".into(), }, @@ -557,6 +593,7 @@ mod tests { key: [7; 32], decision, reminder: Reminder::Created { + provider: MailProvider::Microsoft, list_id: "list".into(), task_id: "task".into(), }, @@ -572,6 +609,62 @@ mod tests { assert_eq!(records[0].updated, record.updated); } } + + #[test] + fn legacy_reminder_bytes_decode_as_microsoft_and_google_tags_round_trip() { + let mut legacy = MAGIC.to_vec(); + legacy.extend_from_slice(&[0; 32]); + legacy.push(1); + legacy.extend_from_slice(&[7; 32]); + legacy.push(1); + legacy.push(2); + legacy.extend_from_slice(&1_i64.to_le_bytes()); + legacy.extend_from_slice(&[4, b'l', b'i', b's', b't']); + legacy.extend_from_slice(&[4, b't', b'a', b's', b'k']); + let (_, records) = decode(&legacy).unwrap(); + assert_eq!( + records[0].reminder, + Reminder::Created { + provider: MailProvider::Microsoft, + list_id: "list".into(), + task_id: "task".into(), + } + ); + + for (expected_tag, reminder) in [ + ( + 4, + Reminder::Created { + provider: MailProvider::Google, + list_id: "list".into(), + task_id: "task".into(), + }, + ), + ( + 5, + Reminder::Completed { + provider: MailProvider::Google, + list_id: "list".into(), + task_id: "task".into(), + }, + ), + ] { + let record = Record { + key: [8; 32], + decision: Decision::Mine, + reminder, + updated: 2, + }; + let bytes = encode(&[0; 32], std::slice::from_ref(&record)).unwrap(); + assert_eq!(bytes[MAGIC.len() + 33 + 33], expected_tag); + assert_eq!(decode(&bytes).unwrap().1[0].reminder, record.reminder); + } + + for invalid in [6, u8::MAX] { + legacy[MAGIC.len() + 33 + 33] = invalid; + assert!(decode(&legacy).is_err()); + } + } #[test] fn fingerprint_is_insensitive_to_case_punctuation_and_incidental_whitespace() { // The concrete, guaranteed improvement this fix makes: two scans that diff --git a/crates/openloops-desktop/src/review_model.rs b/crates/openloops-desktop/src/review_model.rs index c7e2a71..d7fdbb0 100644 --- a/crates/openloops-desktop/src/review_model.rs +++ b/crates/openloops-desktop/src/review_model.rs @@ -7,6 +7,7 @@ use crate::deadline_view::{DeadlineView, classify, classify_with_hint}; use crate::link_state::{LinkKind, Relations}; use crate::loop_state::{Decision, Decisions, Record, Reminder, now}; use openloops_graph::live::{ + MailProvider, reminders::ReminderRequest, review::{LoadProgress, SourceReview}, }; @@ -15,9 +16,9 @@ use std::sync::atomic::Ordering; #[path = "review_scan.rs"] mod scanning; pub(crate) use scanning::{ - ConversationFailure, FailureReason, IncrementalScope, PriorUpdate, ReviewMessage, ScanOptions, - ScanProgress, ScanResult, ScanScope, closure_candidates, compare_decisions, probe, - same_thread_closure_candidates, scan, + ConversationFailure, ConversationKey, FailureReason, IncrementalScope, PriorUpdate, + ReviewMessage, ScanOptions, ScanProgress, ScanResult, ScanScope, closure_candidates, + compare_decisions, probe, same_thread_closure_candidates, scan, }; pub(crate) const SHOW_HANDLED_LABEL: &str = "Show resolved, handled, and dismissed"; @@ -189,9 +190,9 @@ pub struct ReviewState { pub(crate) scan_failed: bool, source_notes: BTreeMap>, source_issue_counts: BTreeMap, - conversation_quality: BTreeMap, - conversation_notes_by_id: BTreeMap>, - conversation_rejection_reasons: BTreeMap>, + conversation_quality: BTreeMap, + conversation_notes_by_id: BTreeMap>, + conversation_rejection_reasons: BTreeMap>, closure_pass_failure: Option, pub(crate) linked_card_titles: BTreeMap<[u8; 32], String>, } @@ -329,11 +330,16 @@ impl ReviewState { /// Returns both appended conversations and existing conversations re-keyed by /// `merge_threads`: a bridging message can absorb an old thread, whose stale /// scan items must then be replaced by [`ReviewState::merge_scan`]. - pub fn append_sources(&mut self, sources: Vec) -> BTreeSet { - let previous_conversations: BTreeMap = self + pub fn append_sources(&mut self, sources: Vec) -> BTreeSet { + let previous_conversations: BTreeMap = self .messages .iter() - .map(|message| (message.input.handle.clone(), message.conversation.clone())) + .map(|message| { + ( + message.input.handle.clone(), + (message.account.clone(), message.conversation.clone()), + ) + }) .collect(); let mut appended_handles = BTreeSet::new(); for source in sources { @@ -410,17 +416,24 @@ impl ReviewState { appended_handles.contains(&message.input.handle) || previous_conversations .get(&message.input.handle) - .is_some_and(|old| old != &message.conversation) + .is_some_and(|old| { + old.0.as_str() != message.account + || old.1.as_str() != message.conversation + }) }) - .map(|message| message.conversation.clone()) + .map(|message| (message.account.clone(), message.conversation.clone())) .collect() } - pub(crate) fn prior_open_items(&self, changed: &BTreeSet) -> Vec { + pub(crate) fn prior_open_items(&self, changed: &BTreeSet) -> Vec { let Some(analysis) = &self.analysis else { return Vec::new(); }; let cards = self.card_contexts(&analysis.items); + let changed: BTreeSet<(&str, &str)> = changed + .iter() + .map(|(account, conversation)| (account.as_str(), conversation.as_str())) + .collect(); analysis .items .iter() @@ -433,7 +446,12 @@ impl ReviewState { .messages .iter() .find(|message| message.input.handle == item.evidence.message) - .is_some_and(|message| !changed.contains(&message.conversation)) + .is_some_and(|message| { + !changed.contains(&( + message.account.as_str(), + message.conversation.as_str(), + )) + }) }) .map(|(item, _)| item.clone()) .collect() @@ -443,7 +461,7 @@ impl ReviewState { &mut self, source_keys: &BTreeSet, sources: Vec, - ) -> BTreeSet { + ) -> BTreeSet { for key in source_keys { self.failed_sources.remove(key); self.source_notes.remove(key); @@ -531,7 +549,7 @@ impl ReviewState { fn merge_conversation_metadata( &mut self, result: &mut ScanResult, - retried: &BTreeSet, + retried: &BTreeSet, ) -> MergedConversationMetadata { let keyed_result_notes: Vec<&String> = result.conversation_notes_by_id.values().flatten().collect(); @@ -552,17 +570,21 @@ impl ReviewState { .append(&mut result.conversation_notes_by_id); self.conversation_rejection_reasons .append(&mut result.conversation_rejection_reasons); - let live: BTreeSet<&str> = self + let live: BTreeSet<(&str, &str)> = self .messages .iter() - .map(|message| message.conversation.as_str()) + .map(|message| (message.account.as_str(), message.conversation.as_str())) .collect(); - self.conversation_quality - .retain(|conversation, _| live.contains(conversation.as_str())); - self.conversation_notes_by_id - .retain(|conversation, _| live.contains(conversation.as_str())); + self.conversation_quality.retain(|conversation, _| { + live.contains(&(conversation.0.as_str(), conversation.1.as_str())) + }); + self.conversation_notes_by_id.retain(|conversation, _| { + live.contains(&(conversation.0.as_str(), conversation.1.as_str())) + }); self.conversation_rejection_reasons - .retain(|conversation, _| live.contains(conversation.as_str())); + .retain(|conversation, _| { + live.contains(&(conversation.0.as_str(), conversation.1.as_str())) + }); let (rejected, degraded) = self .conversation_quality .values() @@ -597,13 +619,23 @@ impl ReviewState { } /// Merges a subset scan into the existing analysis without disturbing untouched cards. - pub fn merge_scan(&mut self, mut result: ScanResult, retried: &BTreeSet) -> usize { + pub fn merge_scan( + &mut self, + mut result: ScanResult, + retried: &BTreeSet, + ) -> usize { self.scan_failed = false; let metadata = self.merge_conversation_metadata(&mut result, retried); + let retried: BTreeSet<(&str, &str)> = retried + .iter() + .map(|(account, conversation)| (account.as_str(), conversation.as_str())) + .collect(); let retried_handles: BTreeSet = self .messages .iter() - .filter(|message| retried.contains(&message.conversation)) + .filter(|message| { + retried.contains(&(message.account.as_str(), message.conversation.as_str())) + }) .map(|message| message.input.handle.clone()) .collect(); let analysis = self.analysis.get_or_insert_with(|| LoopItems { @@ -629,7 +661,8 @@ impl ReviewState { .collect(); let mut kept_lines = Vec::new(); self.failed_conversations_detail.retain(|failure| { - let keep = !retried.contains(&failure.conversation); + let keep = + !retried.contains(&(failure.account.as_str(), failure.conversation.as_str())); if keep && let Some((_, line)) = old_lines.iter().find(|(candidate, _)| candidate == failure) @@ -672,15 +705,17 @@ impl ReviewState { not_started_conversations, closure_pass_failure: self.closure_pass_failure.clone(), }; - let failed_ids: BTreeSet<&str> = self + let failed_ids: BTreeSet<(&str, &str)> = self .failed_conversations_detail .iter() - .map(|failure| failure.conversation.as_str()) + .map(|failure| (failure.account.as_str(), failure.conversation.as_str())) .collect(); merged.analyzed = self .messages .iter() - .filter(|message| !failed_ids.contains(message.conversation.as_str())) + .filter(|message| { + !failed_ids.contains(&(message.account.as_str(), message.conversation.as_str())) + }) .count(); scanning::close_passed_events(&mut merged, &self.messages, chrono::Utc::now().timestamp()); let model = self.analysis_model.clone(); @@ -705,7 +740,7 @@ impl ReviewState { } #[must_use] - pub fn retryable_conversations(&self) -> BTreeSet { + pub fn retryable_conversations(&self) -> BTreeSet { self.failed_conversations_detail .iter() .filter(|failure| { @@ -714,7 +749,7 @@ impl ReviewState { FailureReason::RateLimited | FailureReason::Quota ) }) - .map(|failure| failure.conversation.clone()) + .map(|failure| (failure.account.clone(), failure.conversation.clone())) .collect() } /// Reverts the implied-tracking change from opening a reminder draft @@ -1072,7 +1107,12 @@ impl ReviewState { ) { return None; } - let Reminder::Created { list_id, task_id } = &card.record.reminder else { + let Reminder::Created { + provider: MailProvider::Microsoft, + list_id, + task_id, + } = &card.record.reminder + else { return None; }; if list_id.is_empty() || task_id.is_empty() { @@ -1821,6 +1861,7 @@ pub fn layout_fixture() -> ReviewState { key: first_key, decision: Decision::Mine, reminder: Reminder::Created { + provider: openloops_graph::live::MailProvider::Microsoft, list_id: "list".into(), task_id: "task".into(), }, @@ -1852,6 +1893,7 @@ mod tests { fn failure(conversation: &str, reason: FailureReason) -> ConversationFailure { ConversationFailure { + account: "synthetic-account".into(), conversation: conversation.into(), subject_short: "Synthetic subject".into(), reason, @@ -1863,6 +1905,7 @@ mod tests { messages: Vec, ) -> SourceReview { SourceReview { + provider: openloops_graph::live::MailProvider::Microsoft, label: label.into(), messages, errors: vec![], @@ -1924,7 +1967,10 @@ mod tests { vec!["p1@example.invalid", "p2@example.invalid"], ); let changed = state.append_sources(vec![source_review("Inbox", vec![bridge])]); - assert_eq!(changed, BTreeSet::from(["cA".into()])); + assert_eq!( + changed, + BTreeSet::from([("synthetic-account".into(), "cA".into())]) + ); assert!( state .messages @@ -2000,6 +2046,7 @@ mod tests { #[test] fn appended_source_messages_are_deduplicated_by_account_and_id() { let source = SourceReview { + provider: openloops_graph::live::MailProvider::Microsoft, label: "Personal mailbox / Inbox".into(), messages: vec![openloops_graph::live::review::MailItem { id: "synthetic-id".into(), @@ -2113,7 +2160,10 @@ mod tests { retry.total = 2; retry.conversation_count = 1; retry.analyzed_conversations = 1; - state.merge_scan(retry, &BTreeSet::from(["synthetic-thread".into()])); + state.merge_scan( + retry, + &BTreeSet::from([("synthetic".into(), "synthetic-thread".into())]), + ); let items = &state.analysis.as_ref().unwrap().items; assert!( items @@ -2163,7 +2213,10 @@ mod tests { reminder: Reminder::None, updated: now(), }); - let prior = state.prior_open_items(&BTreeSet::from(["synthetic-thread-3".into()])); + let prior = state.prior_open_items(&BTreeSet::from([( + "synthetic".into(), + "synthetic-thread-3".into(), + )])); assert_eq!(prior.len(), 1); assert_eq!(prior[0].evidence.message, "m1"); } @@ -2217,21 +2270,18 @@ mod tests { #[test] fn merge_scan_drops_metadata_for_absorbed_conversation_ids() { let mut state = layout_fixture(); - state.conversation_quality.insert("absorbed".into(), (1, 1)); + let absorbed = ("synthetic".into(), "absorbed".into()); + state.conversation_quality.insert(absorbed.clone(), (1, 1)); state .conversation_notes_by_id - .insert("absorbed".into(), vec!["Synthetic note".into()]); + .insert(absorbed.clone(), vec!["Synthetic note".into()]); state .conversation_rejection_reasons - .insert("absorbed".into(), vec!["Synthetic reason"]); + .insert(absorbed.clone(), vec!["Synthetic reason"]); state.merge_scan(empty_scan_result(), &BTreeSet::new()); - assert!(!state.conversation_quality.contains_key("absorbed")); - assert!(!state.conversation_notes_by_id.contains_key("absorbed")); - assert!( - !state - .conversation_rejection_reasons - .contains_key("absorbed") - ); + assert!(!state.conversation_quality.contains_key(&absorbed)); + assert!(!state.conversation_notes_by_id.contains_key(&absorbed)); + assert!(!state.conversation_rejection_reasons.contains_key(&absorbed)); } #[test] @@ -2267,7 +2317,10 @@ mod tests { retry.analyzed = 1; retry.conversation_count = 1; retry.analyzed_conversations = 1; - state.merge_scan(retry, &BTreeSet::from(["failed".into()])); + state.merge_scan( + retry, + &BTreeSet::from([("synthetic-account".into(), "failed".into())]), + ); assert_eq!( state.scan_summary, "Reviewed 1 of 1 loaded messages (1 of 1 conversations)." @@ -2306,9 +2359,10 @@ mod tests { initial.conversation_count = 1; initial.analyzed_conversations = 1; initial.analysis.rejected = 1; - initial - .conversation_quality - .insert("conversation-a".into(), (1, 0)); + initial.conversation_quality.insert( + ("synthetic-account".into(), "conversation-a".into()), + (1, 0), + ); state.set_scan(initial, "synthetic-model".into()); assert!(state.scan_incomplete); @@ -2317,10 +2371,14 @@ mod tests { retry.analyzed = 1; retry.conversation_count = 1; retry.analyzed_conversations = 1; - retry - .conversation_quality - .insert("conversation-a".into(), (0, 0)); - state.merge_scan(retry, &BTreeSet::from(["conversation-a".into()])); + retry.conversation_quality.insert( + ("synthetic-account".into(), "conversation-a".into()), + (0, 0), + ); + state.merge_scan( + retry, + &BTreeSet::from([("synthetic-account".into(), "conversation-a".into())]), + ); assert_eq!(state.analysis.as_ref().unwrap().rejected, 0); assert!(!state.scan_incomplete); @@ -2362,15 +2420,17 @@ mod tests { initial.analyzed_conversations = 2; initial.analysis.rejected = 1; initial.analysis.rejection_reasons = vec![reason]; - initial - .conversation_quality - .insert("conversation-b".into(), (1, 0)); - initial - .conversation_rejection_reasons - .insert("conversation-b".into(), vec![reason]); + initial.conversation_quality.insert( + ("synthetic-account".into(), "conversation-b".into()), + (1, 0), + ); + initial.conversation_rejection_reasons.insert( + ("synthetic-account".into(), "conversation-b".into()), + vec![reason], + ); initial.conversation_notes = vec!["Synthetic conversation B note".into()]; initial.conversation_notes_by_id.insert( - "conversation-b".into(), + ("synthetic-account".into(), "conversation-b".into()), vec!["Synthetic conversation B note".into()], ); state.set_scan(initial, "synthetic-model".into()); @@ -2380,10 +2440,14 @@ mod tests { retry.analyzed = 1; retry.conversation_count = 1; retry.analyzed_conversations = 1; - retry - .conversation_quality - .insert("conversation-a".into(), (0, 0)); - state.merge_scan(retry, &BTreeSet::from(["conversation-a".into()])); + retry.conversation_quality.insert( + ("synthetic-account".into(), "conversation-a".into()), + (0, 0), + ); + state.merge_scan( + retry, + &BTreeSet::from([("synthetic-account".into(), "conversation-a".into())]), + ); assert_eq!( state @@ -2396,6 +2460,80 @@ mod tests { ); } + #[test] + fn retry_merge_keeps_same_conversation_id_metadata_separate_by_account() { + let mut state = ReviewState::default(); + for (index, account) in ["account-a", "account-b"].into_iter().enumerate() { + state.messages.push( + scanning::prepare( + &openloops_graph::live::review::MailItem { + id: format!("synthetic-{index}"), + account: account.into(), + conversation: "shared-conversation".into(), + subject: "Synthetic subject".into(), + body: "Synthetic body".into(), + received: "2026-09-01T12:00:00Z".into(), + ..Default::default() + }, + "Inbox", + index, + ) + .unwrap(), + ); + } + let account_a = ("account-a".into(), "shared-conversation".into()); + let account_b = ("account-b".into(), "shared-conversation".into()); + let mut initial = empty_scan_result(); + initial + .conversation_quality + .insert(account_a.clone(), (1, 0)); + initial + .conversation_quality + .insert(account_b.clone(), (0, 1)); + initial + .conversation_notes_by_id + .insert(account_a.clone(), vec!["Account A old note".into()]); + initial + .conversation_notes_by_id + .insert(account_b.clone(), vec!["Account B note".into()]); + initial + .conversation_rejection_reasons + .insert(account_a.clone(), vec!["Account A old reason"]); + initial + .conversation_rejection_reasons + .insert(account_b.clone(), vec!["Account B reason"]); + state.set_scan(initial, "synthetic-model".into()); + + let mut retry = empty_scan_result(); + retry.conversation_quality.insert(account_a.clone(), (0, 2)); + retry + .conversation_notes_by_id + .insert(account_a.clone(), vec!["Account A new note".into()]); + retry + .conversation_rejection_reasons + .insert(account_a.clone(), vec!["Account A new reason"]); + state.merge_scan(retry, &BTreeSet::from([account_a.clone()])); + + assert_eq!(state.conversation_quality[&account_a], (0, 2)); + assert_eq!(state.conversation_quality[&account_b], (0, 1)); + assert_eq!( + state.conversation_notes_by_id[&account_a], + ["Account A new note"] + ); + assert_eq!( + state.conversation_notes_by_id[&account_b], + ["Account B note"] + ); + assert_eq!( + state.conversation_rejection_reasons[&account_a], + ["Account A new reason"] + ); + assert_eq!( + state.conversation_rejection_reasons[&account_b], + ["Account B reason"] + ); + } + #[test] fn retry_summary_partitions_analyzed_failed_and_not_started_conversations() { let mut state = ReviewState::default(); @@ -2449,7 +2587,10 @@ mod tests { ]; state.merge_scan( retry, - &BTreeSet::from(["failed".into(), "not-started".into()]), + &BTreeSet::from([ + ("synthetic-account".into(), "failed".into()), + ("synthetic-account".into(), "not-started".into()), + ]), ); let (analyzed, failed, not_started) = @@ -2708,6 +2849,7 @@ mod tests { #[test] fn reminder_button_enabled_without_a_reminder_or_an_open_draft() { let created = || Reminder::Created { + provider: openloops_graph::live::MailProvider::Microsoft, list_id: "list".into(), task_id: "task".into(), }; @@ -2727,6 +2869,25 @@ mod tests { } } + #[test] + fn google_reminders_are_not_eligible_for_microsoft_sync() { + let mut state = layout_fixture(); + let record = state + .decisions + .records + .iter_mut() + .find(|record| matches!(record.reminder, Reminder::Created { .. })) + .unwrap(); + let Reminder::Created { provider, .. } = &mut record.reminder else { + unreachable!(); + }; + *provider = MailProvider::Google; + + let items = &state.analysis.as_ref().unwrap().items; + let cards = state.card_contexts(items); + assert!(state.reminder_sync_checks(items, &cards).is_empty()); + } + #[test] fn decision_after_setting_reminder_implies_tracking_except_when_watching() { for (current, expected) in [ @@ -3297,6 +3458,7 @@ the scan stopped after a provider error." }; let sources = vec![ SourceReview { + provider: openloops_graph::live::MailProvider::Microsoft, label: "Inbox".into(), messages: vec![ mail("i-1", "c1", 1, "Please send the draft."), @@ -3308,6 +3470,7 @@ the scan stopped after a provider error." failed: false, }, SourceReview { + provider: openloops_graph::live::MailProvider::Microsoft, label: "Sent".into(), messages: vec![mail("s-1", "c3", 3, "Here is the draft.")], errors: vec![], diff --git a/crates/openloops-desktop/src/review_scan.rs b/crates/openloops-desktop/src/review_scan.rs index fbfa469..62b08c4 100644 --- a/crates/openloops-desktop/src/review_scan.rs +++ b/crates/openloops-desktop/src/review_scan.rs @@ -16,7 +16,7 @@ use openloops_domain::deadline_parse::{ DEFAULT_EOD_SECONDS_SINCE_MIDNIGHT, ParseContext, TemporalKind as DeadlineTemporalKind, TimezoneContext, Weekday, reparse, }; -use openloops_graph::live::{ConnectionError, review::MailItem}; +use openloops_graph::live::{ConnectionError, MailProvider, review::MailItem}; use openloops_inference::{ analysis::{ AcceptedClaim, ClaimAnalysis, ReviewEvidence, analyze_claims, analyze_claims_omitting, @@ -41,7 +41,7 @@ use openloops_inference::{ }, walker::canonicalize_html, }; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::io::BufRead; use std::path::Path; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -50,6 +50,7 @@ use std::time::{Duration, Instant}; #[derive(Clone)] pub struct ReviewMessage { + pub provider: MailProvider, pub input: ConversationMessage, pub source: String, pub id: String, @@ -1639,11 +1640,11 @@ pub struct ScanResult { /// by [`probe`]. pub conversation_notes: Vec, /// Rejected and degraded counts attributable to each analyzed conversation. - pub conversation_quality: BTreeMap, + pub conversation_quality: BTreeMap, /// Display notes attributable to each analyzed conversation. - pub conversation_notes_by_id: BTreeMap>, + pub conversation_notes_by_id: BTreeMap>, /// Validator rejection reasons attributable to each analyzed conversation. - pub conversation_rejection_reasons: BTreeMap>, + pub conversation_rejection_reasons: BTreeMap>, /// Number of open loops the closure pass (`scan_closures`) attached a /// pending [`SuggestedUpdate`] to. pub suggested_updates: usize, @@ -1684,20 +1685,22 @@ pub struct PriorUpdate { pub update: SuggestedUpdate, } +pub type ConversationKey = (String, String); + pub enum ScanScope<'a> { Full, - Conversations(&'a BTreeSet), + Conversations(&'a BTreeSet), Incremental(IncrementalScope<'a>), } pub struct IncrementalScope<'a> { - pub conversations: &'a BTreeSet, + pub conversations: &'a BTreeSet, pub new_messages: &'a BTreeSet, pub prior_items: Vec, } impl ScanScope<'_> { - fn filter(&self) -> Option<&BTreeSet> { + fn filter(&self) -> Option<&BTreeSet> { match self { Self::Full => None, Self::Conversations(conversations) => Some(conversations), @@ -1706,6 +1709,15 @@ impl ScanScope<'_> { } } +fn borrowed_filter<'a>(scope: &'a ScanScope<'a>) -> Option> { + scope.filter().map(|filter| { + filter + .iter() + .map(|(account, conversation)| (account.as_str(), conversation.as_str())) + .collect() + }) +} + #[derive(Clone, Copy)] struct ClosureScope<'a> { prior_start: usize, @@ -1720,6 +1732,7 @@ impl ClosureScope<'_> { #[derive(Clone, Debug, PartialEq, Eq)] pub struct ConversationFailure { + pub account: String, pub conversation: String, pub subject_short: String, pub reason: FailureReason, @@ -2198,6 +2211,7 @@ pub fn prepare( let (from_user, recipient) = user_relation(item); let other_addresses = other_addresses(item); Ok(ReviewMessage { + provider: item.provider, input: ConversationMessage { handle: format!("m{index}"), timestamp, @@ -3115,13 +3129,15 @@ fn skipped_triage_conversations( fn triage_pass( messages: &[ReviewMessage], progress: &ScanProgress, - conversation_filter: Option<&BTreeSet>, + conversation_filter: Option<&HashSet<(&str, &str)>>, decision_client: &dyn DecisionClient, ) -> Result { let selected: Vec<&ReviewMessage> = messages .iter() .filter(|message| { - conversation_filter.is_none_or(|filter| filter.contains(&message.conversation)) + conversation_filter.is_none_or(|filter| { + filter.contains(&(message.account.as_str(), message.conversation.as_str())) + }) }) .collect(); let jobs = triage_jobs(&selected); @@ -3255,11 +3271,14 @@ pub fn scan( progress: &ScanProgress, scope: ScanScope<'_>, ) -> Result { - let conversation_filter = scope.filter(); + let conversation_filter = borrowed_filter(&scope); + let conversation_filter = conversation_filter.as_ref(); let selected_messages = messages .iter() .filter(|message| { - conversation_filter.is_none_or(|filter| filter.contains(&message.conversation)) + conversation_filter.is_none_or(|filter| { + filter.contains(&(message.account.as_str(), message.conversation.as_str())) + }) }) .count(); let decision_key = (options.provider == Provider::OpenRouter && options.use_decision_model) @@ -3376,7 +3395,7 @@ fn run_primary_pass( messages: &[ReviewMessage], progress: &ScanProgress, pass: &ParallelPass, - conversation_filter: Option<&BTreeSet>, + conversation_filter: Option<&HashSet<(&str, &str)>>, selected_messages: usize, ) -> Result { let PrimaryPassClients { @@ -3392,11 +3411,12 @@ fn run_primary_pass( let primary_messages: Vec = messages .iter() .filter(|message| { - conversation_filter.is_none_or(|filter| filter.contains(&message.conversation)) - && !extraction - .triage - .skipped - .contains(&(message.account.clone(), message.conversation.clone())) + conversation_filter.is_none_or(|filter| { + filter.contains(&(message.account.as_str(), message.conversation.as_str())) + }) && !extraction + .triage + .skipped + .contains(&(message.account.clone(), message.conversation.clone())) }) .cloned() .collect(); @@ -3431,10 +3451,11 @@ fn run_primary_pass( let primary_messages: Vec = messages .iter() .filter(|message| { - conversation_filter.is_none_or(|filter| filter.contains(&message.conversation)) - && !triage - .skipped - .contains(&(message.account.clone(), message.conversation.clone())) + conversation_filter.is_none_or(|filter| { + filter.contains(&(message.account.as_str(), message.conversation.as_str())) + }) && !triage + .skipped + .contains(&(message.account.clone(), message.conversation.clone())) }) .cloned() .collect(); @@ -3470,7 +3491,7 @@ fn run_primary_pass( fn apply_triage_coverage( result: &mut ScanResult, messages: &[ReviewMessage], - conversation_filter: Option<&BTreeSet>, + conversation_filter: Option<&HashSet<(&str, &str)>>, triage: &TriageResult, selected_messages: usize, ) { @@ -3496,15 +3517,16 @@ fn apply_triage_coverage( result.conversation_notes.push(note.clone()); result .conversation_notes_by_id - .entry(first.conversation.clone()) + .entry((first.account.clone(), first.conversation.clone())) .or_default() .push(note); result .conversation_quality - .insert(first.conversation.clone(), (0, 0)); - result - .conversation_rejection_reasons - .insert(first.conversation.clone(), Vec::new()); + .insert((first.account.clone(), first.conversation.clone()), (0, 0)); + result.conversation_rejection_reasons.insert( + (first.account.clone(), first.conversation.clone()), + Vec::new(), + ); } let mut note = format!( "{} conversations skipped by triage, {} boilerplate paragraphs dropped, {} triage requests skipped (rate limit / errors).", @@ -4019,13 +4041,15 @@ fn extraction_waiting_party_catalog(selected: &[&ReviewMessage]) -> Vec>, + conversation_filter: Option<&HashSet<(&str, &str)>>, decision_client: &dyn DecisionClient, ) -> ExtractionOutcome { let selected: Vec<&ReviewMessage> = messages .iter() .filter(|message| { - conversation_filter.is_none_or(|filter| filter.contains(&message.conversation)) + conversation_filter.is_none_or(|filter| { + filter.contains(&(message.account.as_str(), message.conversation.as_str())) + }) }) .collect(); let jobs = triage_jobs(&selected); @@ -4316,11 +4340,13 @@ fn conversation_note( /// during analysis is the only owned copy that pass actually needs. fn conversations_by_size<'a>( messages: &'a [ReviewMessage], - conversation_filter: Option<&BTreeSet>, + conversation_filter: Option<&HashSet<(&str, &str)>>, ) -> Vec> { let mut conversations: BTreeMap<(&str, &str), Vec<&ReviewMessage>> = BTreeMap::new(); for m in messages { - if conversation_filter.is_some_and(|filter| !filter.contains(&m.conversation)) { + if conversation_filter + .is_some_and(|filter| !filter.contains(&(m.account.as_str(), m.conversation.as_str()))) + { continue; } conversations @@ -4703,6 +4729,9 @@ fn failure_reason(error: ProviderError) -> FailureReason { fn failure_detail(conversation: &[&ReviewMessage], reason: FailureReason) -> ConversationFailure { ConversationFailure { + account: conversation + .first() + .map_or_else(String::new, |message| message.account.clone()), conversation: conversation .first() .map_or_else(String::new, |message| message.conversation.clone()), @@ -4754,9 +4783,10 @@ fn merge_conversation( ) { match outcome { JobOutcome::Completed(Ok(mut analysis)) => { - let conversation_id = conversation - .first() - .map_or_else(String::new, |message| message.conversation.clone()); + let conversation_key = conversation.first().map_or_else( + || (String::new(), String::new()), + |message| (message.account.clone(), message.conversation.clone()), + ); result.analyzed += conversation.len(); result.analyzed_conversations += 1; correct_recap_attribution(&mut analysis, conversation, None); @@ -4765,17 +4795,17 @@ fn merge_conversation( result.conversation_notes.push(note.clone()); result .conversation_notes_by_id - .entry(conversation_id.clone()) + .entry(conversation_key.clone()) .or_default() .push(note); } result.conversation_quality.insert( - conversation_id.clone(), + conversation_key.clone(), (analysis.rejected, analysis.degraded), ); result .conversation_rejection_reasons - .insert(conversation_id, analysis.rejection_reasons.clone()); + .insert(conversation_key, analysis.rejection_reasons.clone()); result.analysis.items.extend(analysis.items); result.analysis.rejected += analysis.rejected; result.analysis.degraded += analysis.degraded; @@ -4849,7 +4879,7 @@ fn scan_conversations_filtered( messages: &[ReviewMessage], progress: &ScanProgress, pass: &ParallelPass, - conversation_filter: Option<&BTreeSet>, + conversation_filter: Option<&HashSet<(&str, &str)>>, analyze: &(dyn Fn(&[ConversationMessage]) -> Result + Sync), ) -> ScanResult { let ordered: Vec> = conversations_by_size(messages, conversation_filter) @@ -5520,7 +5550,7 @@ fn offered_loops<'a>(items: &[LoopItem], messages: &'a [ReviewMessage]) -> Vec = (&'a str, &'a str); +type BorrowedConversationKey<'a> = (&'a str, &'a str); /// For every conversation with messages that could bear on an offered loop, /// the indexes (into `loops`) of the loops plausibly reachable from it. @@ -5534,9 +5564,9 @@ fn loops_by_conversation<'a>( items: &[LoopItem], messages: &'a [ReviewMessage], scope: Option>, -) -> BTreeMap, Vec> { +) -> BTreeMap, Vec> { let (groups_per_account, address_group_counts) = conversation_group_address_counts(messages); - let mut reach: BTreeMap, Vec> = BTreeMap::new(); + let mut reach: BTreeMap, Vec> = BTreeMap::new(); let new_conversations: BTreeSet<(&str, &str)> = scope .into_iter() .flat_map(|scope| { @@ -5607,10 +5637,10 @@ struct ClosureJob<'a> { /// Turns the reach table into at most [`MAX_CLOSURE_CONVERSATIONS`] jobs, /// busiest conversations first, and reports whether the cap dropped any. fn closure_jobs<'a>( - reach: BTreeMap, Vec>, + reach: BTreeMap, Vec>, messages: &'a [ReviewMessage], ) -> (Vec>, bool) { - let mut ranked: Vec<(ConversationKey<'a>, Vec)> = reach.into_iter().collect(); + let mut ranked: Vec<(BorrowedConversationKey<'a>, Vec)> = reach.into_iter().collect(); ranked.sort_by_key(|(_, loops)| std::cmp::Reverse(loops.len())); let capped = ranked.len() > MAX_CLOSURE_CONVERSATIONS; ranked.truncate(MAX_CLOSURE_CONVERSATIONS); @@ -5825,7 +5855,7 @@ struct DecisionParagraphJob { } fn decision_pairs<'a>( - reach: &BTreeMap, Vec>, + reach: &BTreeMap, Vec>, offered: &[OfferedLoop<'a>], messages: &'a [ReviewMessage], scope: Option>, @@ -6203,8 +6233,8 @@ fn apply_updates(result: &mut ScanResult, best: BTreeMap fn escalated_reach<'a>( pairs: &[DecisionPair<'a>], escalated: &BTreeSet, -) -> BTreeMap, Vec> { - let mut reach: BTreeMap, Vec> = BTreeMap::new(); +) -> BTreeMap, Vec> { + let mut reach: BTreeMap, Vec> = BTreeMap::new(); for &pair_index in escalated { let pair = &pairs[pair_index]; let loops = reach @@ -6702,6 +6732,9 @@ fn union_find_union(parent: &mut [usize], a: usize, b: usize) { #[derive(Default)] struct ThreadGroup { + /// Non-Microsoft providers supply authoritative thread identifiers and + /// must never use Exchange's split-thread repair heuristics. + authoritative: bool, subjects: BTreeSet, addresses: BTreeSet, indices: Vec, @@ -6882,7 +6915,13 @@ fn should_merge_with_rules( ) -> bool { // Neither group's raw subjects carried a calendar-response prefix, and // neither is a group source -- both keep their own thread identity. - if a.has_team || b.has_team || a.has_calendar_prefix || b.has_calendar_prefix { + if a.authoritative + || b.authoritative + || a.has_team + || b.has_team + || a.has_calendar_prefix + || b.has_calendar_prefix + { return false; } // A shared normalized subject that is non-empty and substantial. @@ -6936,6 +6975,7 @@ fn thread_rule_states(messages: &[ReviewMessage]) -> Vec { } group.has_calendar_prefix |= raw_subject_has_calendar_prefix(&raw_subject); group.has_team |= message.input.team; + group.authoritative |= message.provider != MailProvider::Microsoft; group .addresses .extend(message.other_addresses.iter().cloned()); @@ -6961,6 +7001,8 @@ fn thread_rule_states(messages: &[ReviewMessage]) -> Vec { if keys[a].0 != keys[b].0 || values[a].has_team || values[b].has_team + || values[a].authoritative + || values[b].authoritative || values[a].has_calendar_prefix || values[b].has_calendar_prefix || values[a] @@ -7040,6 +7082,7 @@ fn merge_threads_with_rules( if m.input.team { entry.has_team = true; } + entry.authoritative |= m.provider != MailProvider::Microsoft; entry.addresses.extend(m.other_addresses.iter().cloned()); entry.indices.push(i); } @@ -7827,30 +7870,42 @@ mod tests { } #[test] - fn conversation_filter_analyzes_only_selected_conversations() { + fn incremental_filter_qualifies_identical_conversation_ids_by_account() { + let first = synthetic("Synthetic selected body", 0, "shared-thread"); + let mut second = synthetic("Synthetic untouched body", 1, "shared-thread"); + second.account = "other-synthetic-account".into(); + let third = synthetic( + "Synthetic same-account untouched body", + 2, + "untouched-thread", + ); let messages = [ - prepare( - &synthetic("Synthetic selected body", 0, "selected"), - "Inbox", - 0, - ) - .unwrap(), - prepare( - &synthetic("Synthetic untouched body", 1, "untouched"), - "Inbox", - 1, - ) - .unwrap(), + prepare(&first, "Inbox", 0).unwrap(), + prepare(&second, "Inbox", 1).unwrap(), + prepare(&third, "Inbox", 2).unwrap(), ]; let calls = AtomicUsize::new(0); - let filter = BTreeSet::from(["selected".to_string()]); + let analyzed_handles = Mutex::new(Vec::new()); + let conversations = + BTreeSet::from([("synthetic-account".to_string(), "shared-thread".to_string())]); + let new_messages = BTreeSet::new(); + let scope = ScanScope::Incremental(IncrementalScope { + conversations: &conversations, + new_messages: &new_messages, + prior_items: Vec::new(), + }); + let filter = borrowed_filter(&scope).unwrap(); let result = super::scan_conversations_filtered( &messages, &ScanProgress::default(), &ParallelPass::new(1), Some(&filter), - &|_| { + &|conversation| { calls.fetch_add(1, Ordering::Relaxed); + analyzed_handles + .lock() + .unwrap_or_else(PoisonError::into_inner) + .extend(conversation.iter().map(|message| message.handle.clone())); Ok(LoopItems { items: vec![], rejected: 0, @@ -7862,6 +7917,12 @@ mod tests { assert_eq!(calls.load(Ordering::Relaxed), 1); assert_eq!(result.conversation_count, 1); assert_eq!(result.total, 1); + assert_eq!( + *analyzed_handles + .lock() + .unwrap_or_else(PoisonError::into_inner), + ["m0"] + ); } #[test] @@ -11862,6 +11923,23 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li + Sync + 'a; + /// The registry accept threshold for `id`; fixtures read it so a + /// re-derived registry does not invalidate them. + fn registry_accept(id: &str) -> f64 { + Registry::get().question(id).unwrap().accept + } + + /// A probability inside `id`'s registry gray band (escalate..accept). + fn registry_gray(id: &str) -> f64 { + let registered = Registry::get().question(id).unwrap(); + registered.accept.midpoint(registered.escalate) + } + + /// `probability` as the `confidence_micros` a suggested update stores. + fn micros(probability: f64) -> u32 { + format!("{:.0}", probability * 1_000_000.0).parse().unwrap() + } + struct FixedDecisionClient<'a> { answer: Box>, calls: AtomicUsize, @@ -11876,11 +11954,13 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li + Sync + 'a, ) -> Self { + // A gray `closure.outcome` choice, so the noul answers decide. + let gray = registry_gray("closure.outcome"); Self { answer: Box::new(move |index, state| { answer(index, state).map(|values| FixedDecisionValues { probabilities: values.to_vec(), - choice: Some(("none".into(), 0.5, 0.5)), + choice: Some(("none".into(), 0.5, gray)), }) }), calls: AtomicUsize::new(0), @@ -12369,7 +12449,7 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li item.evidence.quote = "Please prepare the packet.".into(); let scoped_state = scoped_event_state(&item); assert!(!has_scoped_event_language(&item)); - let accepted = FixedRuleDecisionClient::noul(0.9); + let accepted = FixedRuleDecisionClient::noul(registry_accept(RULE_SCOPED_EVENT)); let rules = answer_rules( RULE_SCOPED_EVENT, RuleCategory::Event, @@ -12396,7 +12476,7 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li RULE_EVENT_MATCH, RuleCategory::Event, vec![state], - &FixedRuleDecisionClient::noul(0.9), + &FixedRuleDecisionClient::noul(registry_accept(RULE_EVENT_MATCH)), ); assert!(match_event_with_rules("planning request", 0, &[event], Some(&rules)).is_some()); let text_event = EventRef { @@ -12415,8 +12495,8 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li .is_some() ); - for (id, gray) in [(RULE_SCOPED_EVENT, 0.8), (RULE_EVENT_MATCH, 0.69)] { - for probability in [gray, 0.1] { + for id in [RULE_SCOPED_EVENT, RULE_EVENT_MATCH] { + for probability in [registry_gray(id), 0.1] { let state = if id == RULE_SCOPED_EVENT { scoped_state.clone() } else { @@ -12440,7 +12520,7 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li second.action = "Provide the draft".into(); let states = duplicate_rule_states(&[first.clone(), second.clone()]); assert_eq!(states.len(), 1); - let client = FixedRuleDecisionClient::noul(0.9); + let client = FixedRuleDecisionClient::noul(registry_accept(RULE_DUPLICATE_ACTION)); let rules = answer_rules( RULE_DUPLICATE_ACTION, RuleCategory::Duplicate, @@ -12466,7 +12546,7 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li assert_eq!(merge_threads(&mut messages.clone()), 0); let states = thread_rule_states(&messages); assert_eq!(states.len(), 1); - let client = FixedRuleDecisionClient::noul(0.9); + let client = FixedRuleDecisionClient::noul(registry_accept(RULE_THREAD_MERGE)); let rules = answer_rules(RULE_THREAD_MERGE, RuleCategory::Thread, states, &client); assert_eq!(merge_threads_with_rules(&mut messages, Some(&rules)), 1); assert_eq!( @@ -12487,12 +12567,13 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li }); let states = deadline_rule_states(std::slice::from_ref(&item), std::slice::from_ref(&message)); - let client = FixedRuleDecisionClient::new(|_, _| { + let accept = registry_accept(RULE_DEADLINE_KIND); + let client = FixedRuleDecisionClient::new(move |_, _| { Ok(serde_json::json!({ "type":"choice", "choice":"event_tied", "probabilities":{"event_tied":0.9,"soft":0.05,"unknown":0.05}, - "confidence":0.9 + "confidence":accept })) }); let rules = answer_rules(RULE_DEADLINE_KIND, RuleCategory::Deadline, states, &client); @@ -12516,7 +12597,7 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li serde_json::to_vec(&client.state(0)).unwrap(), br#"{"phrase":"before kickoff"}"# ); - for confidence in [0.69, 0.1] { + for confidence in [registry_gray(RULE_DEADLINE_KIND), 0.1] { let client = FixedRuleDecisionClient::new(move |_, _| { Ok(serde_json::json!({ "type":"choice", @@ -12638,7 +12719,10 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li fn triage_all_negative_skips_but_a_gray_band_keeps_the_conversation() { let message = triage_message("Synthetic status only."); let negative = FixedDecisionClient::triage(|_, _| Ok([0.1; 6])); - let gray = FixedDecisionClient::triage(|_, _| Ok([0.47, 0.1, 0.1, 0.1, 0.1, 0.1])); + let gray_probability = registry_gray(TRIAGE_IDS[0]); + let gray = FixedDecisionClient::triage(move |_, _| { + Ok([gray_probability, 0.1, 0.1, 0.1, 0.1, 0.1]) + }); assert_eq!( run_triage(std::slice::from_ref(&message), &negative) @@ -13369,7 +13453,8 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li old.input.timestamp -= 1; all.push(old); let new_messages = BTreeSet::from([new_handle.clone()]); - let decision = FixedDecisionClient::new(|_, _| Ok([0.9, 0.1, 0.1, 0.1])); + let fulfilled = registry_accept("closure.fulfilled"); + let decision = FixedDecisionClient::new(move |_, _| Ok([fulfilled, 0.1, 0.1, 0.1])); let mut result = closure_result(Vec::new()); let prior_start = super::offer_prior_items(&mut result, vec![item]); super::scan_decision_closures( @@ -13412,7 +13497,7 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li #[test] fn scan_scope_filter_matches_previous_conversation_filter_behaviour() { - let conversations = BTreeSet::from(["selected".into()]); + let conversations = BTreeSet::from([("synthetic-account".into(), "selected".into())]); let new_messages = BTreeSet::new(); assert!(ScanScope::Full.filter().is_none()); assert_eq!( @@ -13437,7 +13522,8 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li #[test] fn decision_fulfilled_attaches_the_paragraph_confidence_and_tuned_state_shape() { let (all, item) = cross_thread_fixture(); - let decision = FixedDecisionClient::new(|_, _| Ok([0.81, 0.1, 0.1, 0.1])); + let fulfilled = registry_accept("closure.fulfilled"); + let decision = FixedDecisionClient::new(move |_, _| Ok([fulfilled, 0.1, 0.1, 0.1])); let chat = empty_chat(); let result = run_decision_closures(&all, vec![item], &decision, &chat, &ScanProgress::default()); @@ -13453,14 +13539,16 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li assert_eq!(update.evidence_text, "Sure, let's do it."); assert_eq!(update.source_message, "m1"); assert_eq!(update.source_block, 0); - assert_eq!(update.confidence_micros, 810_000); + assert_eq!(update.confidence_micros, micros(fulfilled)); } #[test] fn accepted_outcome_choice_precedes_the_nouls_and_uses_option_probability() { let (all, item) = cross_thread_fixture(); - let decision = FixedDecisionClient::closure_choice(|_, _| { - Ok(([0.1, 0.99, 0.1, 0.1], "fulfilled", 0.42, 0.9)) + let withdrawn = registry_accept("closure.withdrawn"); + let accept = registry_accept("closure.outcome"); + let decision = FixedDecisionClient::closure_choice(move |_, _| { + Ok(([0.1, withdrawn, 0.1, 0.1], "fulfilled", 0.42, accept)) }); let result = run_decision_closures( &all, @@ -13478,8 +13566,10 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li #[test] fn accepted_none_choice_rejects_the_pair_regardless_of_nouls() { let (all, item) = cross_thread_fixture(); - let decision = FixedDecisionClient::closure_choice(|_, _| { - Ok(([0.99, 0.1, 0.1, 0.1], "none", 0.8, 0.9)) + let fulfilled = registry_accept("closure.fulfilled"); + let accept = registry_accept("closure.outcome"); + let decision = FixedDecisionClient::closure_choice(move |_, _| { + Ok(([fulfilled, 0.1, 0.1, 0.1], "none", 0.8, accept)) }); let chat = empty_chat(); let result = @@ -13492,8 +13582,10 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li #[test] fn gray_outcome_choice_falls_back_to_noul_recombination() { let (all, item) = cross_thread_fixture(); - let decision = FixedDecisionClient::closure_choice(|_, _| { - Ok(([0.81, 0.1, 0.1, 0.1], "none", 0.5, 0.5)) + let fulfilled = registry_accept("closure.fulfilled"); + let gray = registry_gray("closure.outcome"); + let decision = FixedDecisionClient::closure_choice(move |_, _| { + Ok(([fulfilled, 0.1, 0.1, 0.1], "none", 0.5, gray)) }); let result = run_decision_closures( &all, @@ -13505,7 +13597,7 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li let update = result.analysis.items[0].suggested_update.as_ref().unwrap(); assert_eq!(update.kind, SuggestedUpdateKind::Closure); - assert_eq!(update.confidence_micros, 810_000); + assert_eq!(update.confidence_micros, micros(fulfilled)); } #[test] @@ -13626,15 +13718,26 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li #[test] fn compare_prediction_flags_the_registry_gray_band_for_nouls_and_choices() { - let noul_gray = Answer::Noul { probability: 0.47 }; + // The noul label is the 0.5 cut; the gray flag is the registry band. + let gray_probability = registry_gray("triage.asks_recipient"); + let noul_gray = Answer::Noul { + probability: gray_probability, + }; let (value, gray) = compare_prediction("triage.asks_recipient", &noul_gray).unwrap(); - assert_eq!(value, CompareValue::Bool(false)); + assert_eq!(value, CompareValue::Bool(gray_probability >= 0.5)); assert!( gray, - "0.47 sits inside triage.asks_recipient's 0.45..0.5 band" + "the midpoint sits inside triage.asks_recipient's band" ); - let noul_accept = Answer::Noul { probability: 0.9 }; + let noul_negative = Answer::Noul { probability: 0.1 }; + let (value, gray) = compare_prediction("triage.asks_recipient", &noul_negative).unwrap(); + assert_eq!(value, CompareValue::Bool(false)); + assert!(!gray); + + let noul_accept = Answer::Noul { + probability: registry_accept("triage.asks_recipient"), + }; let (_, gray) = compare_prediction("triage.asks_recipient", &noul_accept).unwrap(); assert!(!gray); @@ -13666,13 +13769,17 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li CanonicalBlock::new("Withdrawn paragraph.").unwrap(), ]; all.push(reply); + // Both outcomes accept; the tie value clears both thresholds. + let tie = registry_accept("closure.fulfilled").max(registry_accept("closure.withdrawn")); + let higher = tie.midpoint(1.0); + assert!(higher > tie, "the registry leaves room above the tie"); for (withdrawn, expected) in [ - (0.82, "Withdrawn paragraph."), - (0.8, "Fulfilled paragraph."), + (higher, "Withdrawn paragraph."), + (tie, "Fulfilled paragraph."), ] { let decision = FixedDecisionClient::new(move |_, state| { if state["later"]["paragraph_text"] == "Fulfilled paragraph." { - Ok([0.8, 0.1, 0.1, 0.1]) + Ok([tie, 0.1, 0.1, 0.1]) } else { Ok([0.1, withdrawn, 0.1, 0.1]) } @@ -13693,7 +13800,8 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li #[test] fn gray_band_escalates_only_that_conversation_and_loop_to_chat() { let (all, item) = cross_thread_fixture(); - let decision = FixedDecisionClient::new(|_, _| Ok([0.66, 0.1, 0.1, 0.1])); + let gray = registry_gray("closure.fulfilled"); + let decision = FixedDecisionClient::new(move |_, _| Ok([gray, 0.1, 0.1, 0.1])); let chat = ScriptedClient::new(|_, user| Ok(close_first_loop(user))); let result = run_decision_closures(&all, vec![item], &decision, &chat, &ScanProgress::default()); @@ -13714,7 +13822,8 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li let mut reply = prepare(&reply, "Sent", all.len()).unwrap(); reply.input.message.body_blocks = vec![CanonicalBlock::new(text).unwrap()]; all.push(reply); - let decision = FixedDecisionClient::new(|_, _| Ok([0.1, 0.1, 0.8, 0.1])); + let deadline = registry_accept("closure.deadline_changed"); + let decision = FixedDecisionClient::new(move |_, _| Ok([0.1, 0.1, deadline, 0.1])); let chat = ScriptedClient::new(|_, _| Ok(EMPTY_CLAIMS.to_string())); let result = run_decision_closures(&all, vec![item], &decision, &chat, &ScanProgress::default()); @@ -14392,6 +14501,45 @@ Action Items\nSend Thomas the resources on neurosymbolic AI and the Leavenitz li assert!(messages.iter().all(|m| m.conversation == "c1")); } + #[test] + fn only_microsoft_groups_use_exchange_thread_merge_heuristics() { + let pair = |provider| { + let mut request = request_from( + "sam@example.invalid", + "req-1", + "c1", + "acct", + "Alex and Sam discuss quarterly planning", + ); + let mut reply = reply_to( + "sam@example.invalid", + "reply-1", + "c2", + "acct", + "Re: Alex and Sam discuss quarterly planning", + ); + request.provider = provider; + reply.provider = provider; + vec![ + prepare(&request, "Inbox", 0).unwrap(), + prepare(&reply, "Sent", 1).unwrap(), + ] + }; + + let mut microsoft = pair(MailProvider::Microsoft); + assert_eq!(merge_threads(&mut microsoft), 1); + + let mut google = pair(MailProvider::Google); + assert!( + google + .iter() + .all(|message| message.provider == MailProvider::Google) + ); + assert_eq!(merge_threads(&mut google), 0); + assert_eq!(google[0].conversation, "c1"); + assert_eq!(google[1].conversation, "c2"); + } + #[test] fn same_subject_disjoint_participants_are_not_merged() { let request = request_from( diff --git a/crates/openloops-desktop/src/settings.rs b/crates/openloops-desktop/src/settings.rs index ecd9ec1..6e77c86 100644 --- a/crates/openloops-desktop/src/settings.rs +++ b/crates/openloops-desktop/src/settings.rs @@ -1,5 +1,8 @@ //! Setup preferences are a single current-user Windows Credential Manager record. //! No plaintext files, account identifiers in credential names, or diagnostic payloads. +use std::collections::BTreeSet; + +use openloops_graph::live::MailProvider; use zeroize::Zeroizing; const MAGIC: &[u8] = b"OpenLoopsSetup\x01"; @@ -39,6 +42,29 @@ impl Provider { } } +fn mail_providers_tag(providers: &BTreeSet) -> Result<&'static str, SettingsError> { + let microsoft = providers.contains(&MailProvider::Microsoft); + let google = providers.contains(&MailProvider::Google); + match (microsoft, google) { + (true, false) => Ok("ms"), + (false, true) => Ok("google"), + (true, true) => Ok("ms,google"), + (false, false) => Err(SettingsError::Invalid), + } +} + +fn parse_mail_providers(tag: &str) -> Result, SettingsError> { + match tag { + "ms" => Ok(BTreeSet::from([MailProvider::Microsoft])), + "google" => Ok(BTreeSet::from([MailProvider::Google])), + "ms,google" => Ok(BTreeSet::from([ + MailProvider::Microsoft, + MailProvider::Google, + ])), + _ => Err(SettingsError::Invalid), + } +} + /// The Ollama Cloud plan the account is on, which fixes how many model /// requests may be in flight at once. Ollama Cloud allots concurrent /// request slots per plan; requests past the allotment are queued @@ -133,6 +159,9 @@ impl ExtractionBackend { #[derive(Clone)] pub struct Settings { pub client_id: String, + pub google_client_id: String, + pub google_client_secret: Zeroizing, + pub mail_providers: BTreeSet, pub groups: String, pub shared: String, pub key: Zeroizing, @@ -152,6 +181,9 @@ impl Default for Settings { fn default() -> Self { Self { client_id: String::new(), + google_client_id: String::new(), + google_client_secret: Zeroizing::new(String::new()), + mail_providers: BTreeSet::from([MailProvider::Microsoft]), groups: String::new(), shared: String::new(), key: Zeroizing::new(String::new()), @@ -227,6 +259,7 @@ impl Settings { fn encode(&self) -> Result>, SettingsError> { let parallel = self.openrouter_parallel.to_string(); let decision = self.use_decision_model.to_string(); + let mail_providers = mail_providers_tag(&self.mail_providers)?; let fields = [ self.client_id.as_str(), self.groups.as_str(), @@ -240,6 +273,9 @@ impl Settings { parallel.as_str(), decision.as_str(), self.extraction_backend.tag(), + self.google_client_id.as_str(), + self.google_client_secret.as_str(), + mail_providers, ]; let size = fields .iter() @@ -295,6 +331,15 @@ impl Settings { if !remaining.is_empty() { settings.extraction_backend = ExtractionBackend::parse(&next(&mut remaining)?)?; } + if !remaining.is_empty() { + settings.google_client_id = next(&mut remaining)?; + } + if !remaining.is_empty() { + settings.google_client_secret = Zeroizing::new(next(&mut remaining)?); + } + if !remaining.is_empty() { + settings.mail_providers = parse_mail_providers(&next(&mut remaining)?)?; + } if !remaining.is_empty() { return Err(SettingsError::Invalid); } @@ -403,6 +448,9 @@ mod tests { fn synthetic() -> Settings { Settings { client_id: "00000000-0000-0000-0000-000000000000".into(), + google_client_id: "123-fixture.apps.googleusercontent.com".into(), + google_client_secret: Zeroizing::new("fixture-secret".into()), + mail_providers: BTreeSet::from([MailProvider::Microsoft, MailProvider::Google]), groups: "hello@example.invalid\ninvestors@example.invalid\ncareers@example.invalid" .into(), shared: "shared@example.invalid".into(), @@ -442,6 +490,7 @@ mod tests { } #[test] + #[allow(clippy::too_many_lines)] fn versioned_encoding_rejects_truncation_trailing_data_and_unknown_version() { let encoded = synthetic().encode().unwrap(); let settings = synthetic(); @@ -493,6 +542,10 @@ mod tests { ] .concat(), ); + let extraction_end = decision_end + 4 + settings.extraction_backend.tag().len(); + let google_client_id_end = extraction_end + 4 + settings.google_client_id.len(); + let google_client_secret_end = + google_client_id_end + 4 + settings.google_client_secret.len(); for length in 0..encoded.len() { if length == legacy_end { // A record truncated exactly at the pre-provider boundary is @@ -523,6 +576,35 @@ mod tests { assert_eq!(truncated.extraction_backend, ExtractionBackend::ChatModel); continue; } + if length == extraction_end { + let truncated = Settings::decode(&encoded[..length]).unwrap(); + assert!(truncated.google_client_id.is_empty()); + assert!(truncated.google_client_secret.is_empty()); + assert_eq!( + truncated.mail_providers, + BTreeSet::from([MailProvider::Microsoft]) + ); + continue; + } + if length == google_client_id_end { + let truncated = Settings::decode(&encoded[..length]).unwrap(); + assert_eq!(truncated.google_client_id, settings.google_client_id); + assert!(truncated.google_client_secret.is_empty()); + assert_eq!( + truncated.mail_providers, + BTreeSet::from([MailProvider::Microsoft]) + ); + continue; + } + if length == google_client_secret_end { + let truncated = Settings::decode(&encoded[..length]).unwrap(); + assert_eq!(&*truncated.google_client_secret, "fixture-secret"); + assert_eq!( + truncated.mail_providers, + BTreeSet::from([MailProvider::Microsoft]) + ); + continue; + } assert!(Settings::decode(&encoded[..length]).is_err()); } let mut bad = encoded.to_vec(); @@ -541,6 +623,12 @@ mod tests { assert_eq!(decoded.openrouter_parallel, 64); assert!(decoded.use_decision_model); assert_eq!(decoded.extraction_backend, ExtractionBackend::DecisionModel); + assert_eq!(decoded.google_client_id, settings.google_client_id); + assert_eq!(&*decoded.google_client_secret, "fixture-secret"); + assert_eq!( + decoded.mail_providers, + BTreeSet::from([MailProvider::Microsoft, MailProvider::Google]) + ); } #[test] @@ -549,7 +637,14 @@ mod tests { let encoded = settings.encode().unwrap(); let extraction_field_bytes = 4 + settings.extraction_backend.tag().len(); let decision_field_bytes = 4 + settings.use_decision_model.to_string().len(); - let old_end = encoded.len() - extraction_field_bytes - decision_field_bytes; + let new_mail_field_bytes = 4 + + settings.google_client_id.len() + + 4 + + settings.google_client_secret.len() + + 4 + + mail_providers_tag(&settings.mail_providers).unwrap().len(); + let old_end = + encoded.len() - extraction_field_bytes - decision_field_bytes - new_mail_field_bytes; let decoded = Settings::decode(&encoded[..old_end]).unwrap(); assert!(!decoded.use_decision_model); assert_eq!(decoded.extraction_backend, ExtractionBackend::ChatModel); @@ -568,7 +663,13 @@ mod tests { let settings = synthetic(); let encoded = settings.encode().unwrap(); let extraction_field_bytes = 4 + settings.extraction_backend.tag().len(); - let old_end = encoded.len() - extraction_field_bytes; + let new_mail_field_bytes = 4 + + settings.google_client_id.len() + + 4 + + settings.google_client_secret.len() + + 4 + + mail_providers_tag(&settings.mail_providers).unwrap().len(); + let old_end = encoded.len() - extraction_field_bytes - new_mail_field_bytes; let decoded = Settings::decode(&encoded[..old_end]).unwrap(); assert!(decoded.use_decision_model); assert_eq!(decoded.extraction_backend, ExtractionBackend::ChatModel); @@ -719,9 +820,46 @@ mod tests { ); } + #[test] + fn mail_provider_settings_round_trip_and_reject_unknown_tags() { + for providers in [ + BTreeSet::from([MailProvider::Microsoft]), + BTreeSet::from([MailProvider::Google]), + BTreeSet::from([MailProvider::Microsoft, MailProvider::Google]), + ] { + let settings = Settings { + mail_providers: providers.clone(), + ..synthetic() + }; + let decoded = Settings::decode(&settings.encode().unwrap()).unwrap(); + assert_eq!(decoded.mail_providers, providers); + assert_eq!(decoded.google_client_id, settings.google_client_id); + assert_eq!(decoded.google_client_secret, settings.google_client_secret); + } + assert_eq!( + parse_mail_providers("synthetic"), + Err(SettingsError::Invalid) + ); + let settings = synthetic(); + let encoded = settings.encode().unwrap(); + let old_tag_len = mail_providers_tag(&settings.mail_providers).unwrap().len(); + let mut unknown = encoded[..encoded.len() - 4 - old_tag_len].to_vec(); + unknown.extend_from_slice(&u32::try_from("synthetic".len()).unwrap().to_le_bytes()); + unknown.extend_from_slice(b"synthetic"); + assert_eq!( + Settings::decode(&unknown).err(), + Some(SettingsError::Invalid) + ); + assert_eq!( + mail_providers_tag(&BTreeSet::new()), + Err(SettingsError::Invalid) + ); + } + #[test] fn size_is_bounded_before_serializing_private_values() { let mut settings = synthetic(); + assert!(settings.encode().unwrap().len() < MAX_BYTES); settings.groups = "x".repeat(MAX_BYTES); assert!(matches!(settings.encode(), Err(SettingsError::TooLarge))); assert!(Settings::decode(&vec![0; MAX_BYTES + 1]).is_err()); diff --git a/crates/openloops-desktop/src/slint_review.rs b/crates/openloops-desktop/src/slint_review.rs index 6823f88..20ee05e 100644 --- a/crates/openloops-desktop/src/slint_review.rs +++ b/crates/openloops-desktop/src/slint_review.rs @@ -23,7 +23,7 @@ use crate::{ }, }; use openloops_graph::live::{ - ConnectionConfig, ConnectionError, + ConnectionConfig, ConnectionError, MailProvider, review::{LoadProgress, load_recent_with, load_sources_with}, }; use openloops_inference::blocks::CanonicalBlock; @@ -75,7 +75,7 @@ fn reminder_title(decision: Decision, action: &str) -> String { } const REMINDER_VALIDATION_HINT: &str = "Enter a future local date/time and a title of 3–320 bytes. Ambiguous daylight-saving times need a different time."; -const TODO_URL: &str = "https://to-do.office.com/tasks/"; +const TODO_URL: &str = MailProvider::Microsoft.tasks_url(); #[derive(Clone, Debug, PartialEq, Eq)] struct DraftView { @@ -212,6 +212,7 @@ struct EvidenceView { context: String, subject_note: String, url: String, + url_label: String, } pub(crate) fn sender_label(message: &crate::review_model::ReviewMessage) -> String { @@ -227,14 +228,18 @@ pub(crate) fn sender_label(message: &crate::review_model::ReviewMessage) -> Stri } } -fn gated_outlook_url(url: &str) -> String { - if app_model::is_outlook_link(url) { +fn gated_message_url(url: &str) -> String { + if app_model::is_trusted_message_link(url) { url.to_owned() } else { String::new() } } +fn message_url_label(message: &crate::review_model::ReviewMessage) -> String { + format!("Open message in {}", message.provider.mail_client_name()) +} + /// Wraps a non-empty evidence quote in typographic quotes (Companion §4.4); /// an empty quote stays empty so `!evidence.quote.is-empty` in `review.slint` /// still gates the quote block correctly. @@ -265,7 +270,8 @@ fn evidence_card( anchor.context.clone() }, subject_note: String::new(), - url: message.map_or_else(String::new, |message| gated_outlook_url(&message.web_link)), + url: message.map_or_else(String::new, |message| gated_message_url(&message.web_link)), + url_label: message.map_or_else(String::new, message_url_label), } } @@ -288,7 +294,8 @@ fn event_time_evidence_card( } else { String::new() }, - url: message.map_or_else(String::new, |message| gated_outlook_url(&message.web_link)), + url: message.map_or_else(String::new, |message| gated_message_url(&message.web_link)), + url_label: message.map_or_else(String::new, message_url_label), } } @@ -325,7 +332,8 @@ fn evidence_cards( quote: typographic_quote(&mention.quote), context: String::new(), subject_note: String::new(), - url: message.map_or_else(String::new, |message| gated_outlook_url(&message.web_link)), + url: message.map_or_else(String::new, |message| gated_message_url(&message.web_link)), + url_label: message.map_or_else(String::new, message_url_label), }); } cards @@ -340,6 +348,7 @@ struct CompletionView { quote: String, cross_thread: bool, url: String, + url_label: String, } fn completion_card( @@ -360,6 +369,7 @@ fn completion_card( quote: evidence.quote, cross_thread: item.cross_thread, url: evidence.url, + url_label: evidence.url_label, } } else if item.unverified_resolution { CompletionView { @@ -372,12 +382,13 @@ fn completion_card( quote: String::new(), cross_thread: false, url: String::new(), + url_label: String::new(), } } else { CompletionView { state: "none", label: "No matching completion was identified in the scanned conversation. Work may have happened elsewhere or outside this history window.".into(), - sender: String::new(), time: String::new(), quote: String::new(), cross_thread: false, url: String::new(), + sender: String::new(), time: String::new(), quote: String::new(), cross_thread: false, url: String::new(), url_label: String::new(), } } } @@ -396,6 +407,7 @@ struct SuggestedView { sender: String, time: String, url: String, + url_label: String, can_accept: bool, reject_label: &'static str, } @@ -440,7 +452,8 @@ fn suggested_view(review: &ReviewState, update: Option<&SuggestedUpdate>) -> Sug .unwrap_or_default(), sender: message.map_or_else(String::new, sender_label), time: message.map_or_else(String::new, |message| message.date_label.clone()), - url: message.map_or_else(String::new, |message| gated_outlook_url(&message.web_link)), + url: message.map_or_else(String::new, |message| gated_message_url(&message.web_link)), + url_label: message.map_or_else(String::new, message_url_label), can_accept, reject_label, } @@ -512,7 +525,7 @@ fn conversation_url( message.account == source.account && message.conversation == source.conversation }) .max_by_key(|message| message.input.timestamp) - .map_or_else(String::new, |message| gated_outlook_url(&message.web_link)) + .map_or_else(String::new, |message| gated_message_url(&message.web_link)) } /// The last projected `Vec`/value actually pushed to each of @@ -1482,6 +1495,7 @@ fn sync_review_inner( context: evidence.context.into(), subject_note: evidence.subject_note.into(), url: evidence.url.into(), + url_label: evidence.url_label.into(), }) .collect::>(); sync_list_cached(&mut review_ui.cache.evidence, evidence, |m| { @@ -1495,6 +1509,7 @@ fn sync_review_inner( quote: selected.completion.quote.into(), cross_thread: selected.completion.cross_thread, url: selected.completion.url.into(), + url_label: selected.completion.url_label.into(), }; sync_value_cached(&mut review_ui.cache.completion, completion, |c| { window.set_completion_card(c); @@ -1507,6 +1522,7 @@ fn sync_review_inner( sender: selected.suggested.sender.into(), time: selected.suggested.time.into(), url: selected.suggested.url.into(), + url_label: selected.suggested.url_label.into(), can_accept: selected.suggested.can_accept, reject_label: selected.suggested.reject_label.into(), }; @@ -1608,7 +1624,10 @@ pub(crate) fn scan_mode(review: &ReviewState) -> ScanMode { fn start_mail_load(model: &Rc>, mode: ScanMode) -> Result<(), ConnectionError> { let config = { let model = model.borrow(); - ConnectionConfig::new(model.client_id.trim(), Some(&model.shared)) + let client_id = model + .effective_microsoft_client_id() + .ok_or(ConnectionError::InvalidConfiguration)?; + ConnectionConfig::new(&client_id, Some(&model.shared)) .and_then(|config| config.with_groups(Some(&model.groups)))? }; let mut model_ref = model.borrow_mut(); @@ -1626,7 +1645,7 @@ fn start_mail_load(model: &Rc>, mode: ScanMode) -> Result<(), let progress = std::sync::Arc::new(LoadProgress::default()); let cache = std::sync::Arc::clone(&model_ref.mail_cache); model_ref.load_progress = Some(std::sync::Arc::clone(&progress)); - let signed_in = openloops_graph::live::has_session(); + let signed_in = openloops_graph::live::has_session_for(MailProvider::Microsoft); match mode { ScanMode::Full => model_ref.start( Service::Review, @@ -1693,7 +1712,12 @@ fn apply_reconcile(review: &mut ReviewState, key: [u8; 32], exists: bool) { // address and skips silently, exactly as if no reminder existed for // that purpose; the reminder still counts as Created for every other // purpose (dedup, the pill, the button state). + let provider = match record.reminder { + Reminder::Created { provider, .. } | Reminder::Completed { provider, .. } => provider, + Reminder::None | Reminder::Attempted => MailProvider::Microsoft, + }; Reminder::Created { + provider, list_id: String::new(), task_id: String::new(), } @@ -1707,7 +1731,11 @@ fn dispatch_pending_reminder(model: &mut AppModel) -> bool { let Some((key, request)) = model.review.pending_reminder.take() else { return false; }; - match ConnectionConfig::new(model.client_id.trim(), None) { + match model + .effective_microsoft_client_id() + .ok_or(ConnectionError::InvalidConfiguration) + .and_then(|client_id| ConnectionConfig::new(&client_id, None)) + { Ok(config) => { model.start( Service::Review, @@ -1777,9 +1805,11 @@ fn decide_selected(model: &mut AppModel, selected: Option<[u8; 32]>, value: i32) // line, dispatched after the decision is already saved. let complete_task = if decision == Decision::Done { match &reminder { - Reminder::Created { list_id, task_id } - if !list_id.is_empty() && !task_id.is_empty() => - { + Reminder::Created { + provider: MailProvider::Microsoft, + list_id, + task_id, + } if !list_id.is_empty() && !task_id.is_empty() => { Some((list_id.clone(), task_id.clone())) } _ => None, @@ -1789,7 +1819,8 @@ fn decide_selected(model: &mut AppModel, selected: Option<[u8; 32]>, value: i32) }; model.review.apply_decision_change(key, decision, reminder); if let Some((list_id, task_id)) = complete_task - && let Ok(config) = ConnectionConfig::new(model.client_id.trim(), None) + && let Some(client_id) = model.effective_microsoft_client_id() + && let Ok(config) = ConnectionConfig::new(&client_id, None) { model.start( Service::Review, @@ -1957,8 +1988,13 @@ pub(crate) fn register_callbacks( } else { let config = { let model = model.borrow(); - ConnectionConfig::new(model.client_id.trim(), Some(&model.shared)) - .and_then(|config| config.with_groups(Some(&model.groups))) + model + .effective_microsoft_client_id() + .ok_or(ConnectionError::InvalidConfiguration) + .and_then(|client_id| { + ConnectionConfig::new(&client_id, Some(&model.shared)) + .and_then(|config| config.with_groups(Some(&model.groups))) + }) }; match config { Ok(config) => { @@ -1968,7 +2004,7 @@ pub(crate) fn register_callbacks( model_ref.load_progress = Some(std::sync::Arc::clone(&progress)); model_ref.start( Service::Review, - if openloops_graph::live::has_session() { + if openloops_graph::live::has_session_for(MailProvider::Microsoft) { "Downloading recent messages" } else { "Complete Microsoft sign-in; then downloading recent messages" @@ -2446,6 +2482,7 @@ pub(crate) fn register_callbacks( model_ref.review.pending_reminder = Some(( draft.key, openloops_graph::live::reminders::ReminderRequest { + provider: openloops_graph::live::MailProvider::Microsoft, account: draft.account, title: draft.title.trim().into(), at_utc: at, @@ -2485,7 +2522,11 @@ pub(crate) fn register_callbacks( }); } window.on_open_external(|url| { - if url.as_str() == TODO_URL || app_model::is_outlook_link(&url) { + if MailProvider::ALL + .iter() + .any(|provider| url.as_str() == provider.tasks_url()) + || app_model::is_trusted_message_link(&url) + { let _ = opener::open(url.as_str()); } }); @@ -2521,16 +2562,19 @@ mod tests { app.selected = "synthetic-model".into(); app.review.failed_conversations_detail = vec![ ConversationFailure { + account: "synthetic-account".into(), conversation: "timeout".into(), subject_short: "Synthetic".into(), reason: FailureReason::Timeout, }, ConversationFailure { + account: "synthetic-account".into(), conversation: "rate".into(), subject_short: "Synthetic".into(), reason: FailureReason::RateLimited, }, ConversationFailure { + account: "synthetic-account".into(), conversation: "quota".into(), subject_short: "Synthetic".into(), reason: FailureReason::Quota, @@ -3226,6 +3270,7 @@ mod tests { status_pill_hint( Decision::Done, &Reminder::Completed { + provider: openloops_graph::live::MailProvider::Microsoft, list_id: "list".into(), task_id: "task".into() }, @@ -3660,6 +3705,7 @@ mod tests { key, Decision::Mine, Reminder::Created { + provider: openloops_graph::live::MailProvider::Microsoft, list_id: "list".into(), task_id: "task".into(), }, @@ -3726,6 +3772,7 @@ mod tests { }; assert_eq!(reminder_state_view(&record).state, "none"); record.reminder = Reminder::Created { + provider: openloops_graph::live::MailProvider::Microsoft, list_id: "list".into(), task_id: "task".into(), }; @@ -3781,6 +3828,11 @@ mod tests { .iter() .all(|card| card.url.starts_with("https://outlook.office.com/")) ); + assert!( + cards + .iter() + .all(|card| card.url_label == "Open message in Outlook") + ); } #[test] @@ -3817,25 +3869,41 @@ mod tests { } #[test] - fn outlook_link_projection_rejects_every_other_url() { + fn message_link_projection_accepts_each_provider_and_rejects_every_other_url() { assert_eq!( - gated_outlook_url("https://outlook.office.com/mail/item"), + gated_message_url("https://outlook.office.com/mail/item"), "https://outlook.office.com/mail/item" ); assert_eq!( - gated_outlook_url("https://outlook.office365.com/mail/item"), + gated_message_url("https://outlook.office365.com/mail/item"), "https://outlook.office365.com/mail/item" ); assert_eq!( - gated_outlook_url("https://outlook.live.com/mail/item"), + gated_message_url("https://outlook.live.com/mail/item"), "https://outlook.live.com/mail/item" ); assert_eq!( - gated_outlook_url("https://outlook.office365.us/mail/item"), + gated_message_url("https://outlook.office365.us/mail/item"), "https://outlook.office365.us/mail/item" ); - assert!(gated_outlook_url("https://example.invalid/outlook.office.com/").is_empty()); - assert!(gated_outlook_url("http://outlook.office.com/mail/item").is_empty()); + assert_eq!( + gated_message_url("https://mail.google.com/mail/u/0/#inbox/synthetic"), + "https://mail.google.com/mail/u/0/#inbox/synthetic" + ); + assert!(gated_message_url("https://example.invalid/outlook.office.com/").is_empty()); + assert!(gated_message_url("https://evil.example/mail.google.com/").is_empty()); + assert!(gated_message_url("http://outlook.office.com/mail/item").is_empty()); + } + + #[test] + fn message_link_label_uses_the_messages_provider() { + let mut review = crate::review_model::layout_fixture(); + review.messages[0].provider = MailProvider::Google; + review.messages[0].web_link = "https://mail.google.com/mail/u/0/#inbox/synthetic".into(); + let item = &review.analysis.as_ref().unwrap().items[0]; + let card = evidence_card("Synthetic evidence", &item.evidence, &review.messages); + assert_eq!(card.url_label, "Open message in Gmail"); + assert_eq!(card.url, review.messages[0].web_link); } #[test] @@ -3935,6 +4003,57 @@ mod tests { )); } + #[test] + fn reconcile_preserves_the_existing_google_reminder_provider() { + let mut review = crate::review_model::layout_fixture(); + let record = review + .decisions + .records + .iter_mut() + .find(|record| matches!(record.reminder, Reminder::Created { .. })) + .unwrap(); + let key = record.key; + let Reminder::Created { provider, .. } = &mut record.reminder else { + unreachable!(); + }; + *provider = MailProvider::Google; + + apply_reconcile(&mut review, key, true); + + assert!(matches!( + review.decisions.get(&key).reminder, + Reminder::Created { + provider: MailProvider::Google, + ref list_id, + ref task_id, + } if list_id.is_empty() && task_id.is_empty() + )); + } + + #[test] + fn handled_google_reminder_does_not_start_microsoft_completion() { + let mut app = model(); + app.client_id = "00000000-0000-4000-8000-000000000000".into(); + app.review = crate::review_model::layout_fixture(); + let record = app + .review + .decisions + .records + .iter_mut() + .find(|record| matches!(record.reminder, Reminder::Created { .. })) + .unwrap(); + let key = record.key; + let Reminder::Created { provider, .. } = &mut record.reminder else { + unreachable!(); + }; + *provider = MailProvider::Google; + + decide_selected(&mut app, Some(key), DECISION_DONE); + + assert_eq!(app.review.decisions.get(&key).decision, Decision::Done); + assert!(app.pending.is_none()); + } + fn suggestion(kind: SuggestedUpdateKind, temporal: Option<&str>) -> SuggestedUpdate { SuggestedUpdate { kind, diff --git a/crates/openloops-desktop/src/slint_ui.rs b/crates/openloops-desktop/src/slint_ui.rs index 66c67b7..cb8cd95 100644 --- a/crates/openloops-desktop/src/slint_ui.rs +++ b/crates/openloops-desktop/src/slint_ui.rs @@ -11,7 +11,7 @@ use crate::{ }, training_export, }; -use openloops_graph::live::{ConnectionConfig, check_connection, clear_session}; +use openloops_graph::live::{ConnectionConfig, MailProvider, check_connection, clear_session_for}; use openloops_inference::{ decision::{OpenRouterDecisions, registry::Registry}, ollama::{OllamaCloud, available_models}, @@ -206,8 +206,19 @@ pub(crate) fn sync(model: &AppModel, window: &AppWindow) { // succeeded, or once review messages have already loaded (mail is // plainly being scanned even if a rescan's own check has not yet // re-run), rather than a misleading "Not signed in". - let account = - AccountDisplay::connected(model.microsoft.succeeded || !model.review.messages.is_empty()); + let connected_providers = MailProvider::ALL + .iter() + .copied() + .filter(|provider| { + model.connection_status(*provider).succeeded + || model + .review + .messages + .iter() + .any(|message| message.provider == *provider) + }) + .collect::>(); + let account = AccountDisplay::connected(&connected_providers); let cards = model .review .analysis @@ -235,6 +246,9 @@ pub(crate) fn sync(model: &AppModel, window: &AppWindow) { let (strip_model, scan_chip) = crate::slint_review::scan_strip_view(&strip, model, &cards); window.set_review_scan_chip_text(scan_chip.into()); window.set_account_signed_in(account.signed_in); + window.set_account_text(account.name.into()); + window.set_shared_registration_active(model.shared_registration_active()); + window.set_admin_consent_url(model.admin_consent_url().unwrap_or_default().into()); window.set_review_badge( i32::try_from(open_badge_count(&cards, model.review.show_call_summaries)) .unwrap_or(i32::MAX), @@ -283,7 +297,7 @@ pub(crate) fn sync(model: &AppModel, window: &AppWindow) { // these same two properties every tick without touching anything else on // this screen. window.set_microsoft_busy_line( - if model.pending_service == Service::Microsoft { + if model.pending_service == Service::Mailbox { busy_text.as_deref().unwrap_or_default() } else { "" @@ -362,7 +376,7 @@ pub(crate) fn sync(model: &AppModel, window: &AppWindow) { Provider::OllamaCloud => "Load cloud models".into(), Provider::OpenRouter => "Load ZDR models".into(), }); - window.set_can_check_microsoft(!model.client_id.trim().is_empty()); + window.set_can_check_microsoft(model.effective_microsoft_client_id().is_some()); window.set_can_load_models(match model.provider { Provider::OllamaCloud => !model.key.is_empty(), Provider::OpenRouter => true, @@ -415,7 +429,7 @@ pub(crate) fn sync_busy(model: &AppModel, window: &AppWindow) { } match model.pending_service { - Service::Microsoft => window.set_microsoft_busy_line(busy_text.into()), + Service::Mailbox => window.set_microsoft_busy_line(busy_text.into()), Service::Model => window.set_model_busy_line(busy_text.into()), Service::Review => {} } @@ -650,7 +664,7 @@ pub fn run() -> Result<(), slint::PlatformError> { let (sender, receiver) = std::sync::mpsc::channel::(); std::mem::forget(sender); initial_model.pending = Some(receiver); - initial_model.pending_service = Service::Microsoft; + initial_model.pending_service = Service::Mailbox; initial_model.progress = "Complete Microsoft sign-in; then downloading recent messages"; initial_model.started = std::time::Instant::now() .checked_sub(Duration::from_secs(82)) @@ -824,7 +838,7 @@ pub fn run() -> Result<(), slint::PlatformError> { if !replace_if_changed(&mut model_ref.client_id, value) { return; } - clear_session(); + clear_session_for(MailProvider::Microsoft); model_ref.clear_mail_cache(); clear_microsoft_after_edit(&mut model_ref); drop(model_ref); @@ -845,17 +859,24 @@ pub fn run() -> Result<(), slint::PlatformError> { window.on_check_microsoft(move || { let config = { let model = model.borrow(); - ConnectionConfig::new(model.client_id.trim(), Some(&model.shared)) - .and_then(|config| config.with_groups(Some(&model.groups))) + model + .effective_microsoft_client_id() + .ok_or(openloops_graph::live::ConnectionError::InvalidConfiguration) + .and_then(|client_id| { + ConnectionConfig::new(&client_id, Some(&model.shared)) + .and_then(|config| config.with_groups(Some(&model.groups))) + }) }; match config { Ok(config) => { let mut model_ref = model.borrow_mut(); model_ref.microsoft = Status::default(); model_ref.start( - Service::Microsoft, + Service::Mailbox, "Complete Microsoft sign-in in your browser", - move || Outcome::Microsoft(check_connection(&config)), + move || { + Outcome::Connection(MailProvider::Microsoft, check_connection(&config)) + }, || {}, ); start_timer(&timer); diff --git a/crates/openloops-desktop/ui/app.slint b/crates/openloops-desktop/ui/app.slint index b435609..932b629 100644 --- a/crates/openloops-desktop/ui/app.slint +++ b/crates/openloops-desktop/ui/app.slint @@ -26,6 +26,7 @@ export component AppWindow inherits Window { in property busy-chip-text; in property review-scan-chip-text; in property account-signed-in; + in property account-text; in property review-badge; in property provider-name; in property provider-connected; @@ -35,6 +36,8 @@ export component AppWindow inherits Window { in property store-available; in property settings-status; in property settings-succeeded; + in property shared-registration-active; + in property admin-consent-url; in-out property client-id; in-out property groups; in-out property shared-mailboxes; @@ -174,11 +177,12 @@ export component AppWindow inherits Window { VerticalLayout { spacing: 0px; - TitleBar { scanning: root.scanning; scan-text: root.review-scan-chip-text.is-empty ? root.busy-chip-text : root.review-scan-chip-text; account-signed-in: root.account-signed-in; } + TitleBar { scanning: root.scanning; scan-text: root.review-scan-chip-text.is-empty ? root.busy-chip-text : root.review-scan-chip-text; account-signed-in: root.account-signed-in; account-text: root.account-text; } HorizontalLayout { spacing: 0px; vertical-stretch: 1; NavRail { selected-index <=> root.active-screen; review-badge: root.review-badge; provider-name: root.provider-name; provider-connected: root.provider-connected; enabled: !root.busy; selected(index) => { root.navigate(index); } } if root.active-screen == 1: SourcesScreen { controls-enabled: !root.busy; store-available: root.store-available; settings-status: root.settings-status; settings-succeeded: root.settings-succeeded; + shared-registration-active: root.shared-registration-active; admin-consent-url: root.admin-consent-url; client-id <=> root.client-id; groups <=> root.groups; shared-mailboxes <=> root.shared-mailboxes; own-inbox-accessible: root.own-inbox-accessible; microsoft-lines: root.microsoft-lines; microsoft-busy-line: root.microsoft-busy-line; provider-index <=> root.provider-index; api-key <=> root.api-key; show-key <=> root.show-key; models: root.models; model-index <=> root.model-index; model-value <=> root.model-value; diff --git a/crates/openloops-desktop/ui/chrome.slint b/crates/openloops-desktop/ui/chrome.slint index 99f7eaa..e0c4f7a 100644 --- a/crates/openloops-desktop/ui/chrome.slint +++ b/crates/openloops-desktop/ui/chrome.slint @@ -5,6 +5,7 @@ export component TitleBar inherits Rectangle { in property scanning; in property scan-text; in property account-signed-in; + in property account-text; height: Tokens.title-height; background: Tokens.brand; HorizontalLayout { @@ -35,7 +36,7 @@ export component TitleBar inherits Rectangle { // messages have already loaded, and nothing at all otherwise (never // a misleading "Not signed in" while mail is actually being // scanned). - if root.account-signed-in: Text { text: "Signed in · Microsoft 365"; color: Tokens.bg-surface; font-size: Tokens.font-13; horizontal-alignment: right; vertical-alignment: center; } + if root.account-signed-in: Text { text: root.account-text; color: Tokens.bg-surface; font-size: Tokens.font-13; horizontal-alignment: right; vertical-alignment: center; } } Rectangle { x: 0; y: parent.height - Tokens.hairline; width: 100%; height: Tokens.hairline; background: Tokens.stroke-1; } } diff --git a/crates/openloops-desktop/ui/review.slint b/crates/openloops-desktop/ui/review.slint index 4f27632..17a1d58 100644 --- a/crates/openloops-desktop/ui/review.slint +++ b/crates/openloops-desktop/ui/review.slint @@ -18,9 +18,9 @@ export struct ReviewRow { export struct MetaCell { label: string, value: string } export struct ReviewPill { text: string, kind: string, hint: string } -export struct EvidenceCard { label: string, sender: string, time: string, quote: string, context: string, subject-note: string, url: string } -export struct CompletionCard { state: string, label: string, sender: string, time: string, quote: string, cross-thread: bool, url: string } -export struct SuggestedUpdateCard { state: string, label: string, evidence: string, deadline: string, sender: string, time: string, url: string, can-accept: bool, reject-label: string } +export struct EvidenceCard { label: string, sender: string, time: string, quote: string, context: string, subject-note: string, url: string, url-label: string } +export struct CompletionCard { state: string, label: string, sender: string, time: string, quote: string, cross-thread: bool, url: string, url-label: string } +export struct SuggestedUpdateCard { state: string, label: string, evidence: string, deadline: string, sender: string, time: string, url: string, url-label: string, can-accept: bool, reject-label: string } export struct ConversationRow { initials: string, sender: string, meta: string, body: string, sent: bool, quoted-history: string } export struct ReminderStateView { state: string, text: string, marker: string } export struct ScanStripModel { @@ -390,7 +390,7 @@ export component ReviewScreen { HorizontalLayout { spacing: Tokens.space-2; alignment: start; if root.suggested-update.can-accept: PrimaryButton { text: "Accept"; enabled: !root.busy; clicked => { root.suggested-update-resolved(true); } } SecondaryButton { text: root.suggested-update.reject-label; enabled: !root.busy; clicked => { root.suggested-update-resolved(false); } } - if !root.suggested-update.url.is-empty: LinkButton { text: "Open message in Outlook"; enabled: !root.busy; clicked => { root.open-external(root.suggested-update.url); } } + if !root.suggested-update.url.is-empty: LinkButton { text: root.suggested-update.url-label; enabled: !root.busy; clicked => { root.open-external(root.suggested-update.url); } } } } VerticalLayout { width: 100%; spacing: Tokens.space-2; alignment: start; @@ -559,7 +559,7 @@ export component ReviewScreen { text: "· " + evidence.sender + " · " + evidence.time; color: Tokens.text-2; font-size: Tokens.font-13; wrap: word-wrap; } } - if !evidence.url.is-empty: HorizontalLayout { Rectangle { horizontal-stretch: 1; } LinkButton { text: "Open message in Outlook"; enabled: !root.busy; clicked => { root.open-external(evidence.url); } } } + if !evidence.url.is-empty: HorizontalLayout { Rectangle { horizontal-stretch: 1; } LinkButton { text: evidence.url-label; enabled: !root.busy; clicked => { root.open-external(evidence.url); } } } } } Rectangle { height: Tokens.hairline; background: Tokens.stroke-1; } @@ -623,7 +623,7 @@ export component ReviewScreen { HorizontalLayout { spacing: Tokens.space-2; Rectangle { horizontal-stretch: 1; } if root.completion-card.cross-thread: Pill { text: "evidence in another conversation"; kind: "success"; } - if !root.completion-card.url.is-empty: LinkButton { text: "Open message in Outlook"; enabled: !root.busy; clicked => { root.open-external(root.completion-card.url); } } + if !root.completion-card.url.is-empty: LinkButton { text: root.completion-card.url-label; enabled: !root.busy; clicked => { root.open-external(root.completion-card.url); } } } } } diff --git a/crates/openloops-desktop/ui/sources.slint b/crates/openloops-desktop/ui/sources.slint index 1077246..443e2ac 100644 --- a/crates/openloops-desktop/ui/sources.slint +++ b/crates/openloops-desktop/ui/sources.slint @@ -46,6 +46,8 @@ export component SourcesScreen { in property store-available: true; in property settings-status; in property settings-succeeded; + in property shared-registration-active; + in property admin-consent-url; in-out property client-id; in-out property groups; in-out property shared-mailboxes; diff --git a/crates/openloops-graph/src/live.rs b/crates/openloops-graph/src/live.rs index 4537904..19b0c58 100644 --- a/crates/openloops-graph/src/live.rs +++ b/crates/openloops-graph/src/live.rs @@ -1,11 +1,19 @@ -//! Session-only Microsoft connection. No token or response is persisted. +//! Session-only mail-provider connections. No token or response is persisted. //! The organizations authority supports commercial work/school tenants. //! This module does not enable background processing or reminder writes. +//! Google-specific live support is isolated under [`google`]. mod callback; +pub mod google; mod groups; +pub mod provider; +pub mod registration; pub mod reminders; pub mod review; +#[cfg(test)] +mod test_support; + +pub use provider::{AccountConfig, MailProvider, ProviderLoad, load_all}; use std::collections::BTreeSet; use std::io::Read; @@ -15,8 +23,8 @@ use std::time::{Duration, Instant}; use oauth2::basic::BasicClient; use oauth2::{ - AuthUrl, AuthorizationCode, ClientId, CsrfToken, PkceCodeChallenge, RedirectUrl, Scope, - TokenResponse, TokenUrl, + AuthType, AuthUrl, AuthorizationCode, ClientId, ClientSecret, CsrfToken, PkceCodeChallenge, + RedirectUrl, Scope, TokenResponse, TokenUrl, }; use reqwest::blocking::Client; use url::Url; @@ -26,9 +34,35 @@ const MAX_RESPONSE: u64 = 1024 * 1024; const TOKEN_TIMEOUT_SECONDS: u64 = 30; const GRAPH_TIMEOUT_SECONDS: u64 = 90; static CONNECTING: AtomicBool = AtomicBool::new(false); -static SESSION: Mutex> = Mutex::new(None); +static SESSIONS: Mutex<[Option; 2]> = Mutex::new([None, None]); + +pub(super) struct OAuthEndpoints { + pub authorize: &'static str, + pub token: &'static str, + pub redirect_host: callback::RedirectHost, + pub client_secret: Option, + pub extra_params: &'static [(&'static str, &'static str)], + pub normalize_scope: fn(&str) -> String, +} + +fn microsoft_scope(scope: &str) -> String { + if scope.starts_with("https://graph.microsoft.com/") { + scope.to_owned() + } else { + format!("https://graph.microsoft.com/{scope}") + } +} + +pub(super) const MICROSOFT: OAuthEndpoints = OAuthEndpoints { + authorize: AUTHORIZE, + token: TOKEN, + redirect_host: callback::RedirectHost::Localhost, + client_secret: None, + extra_params: &[("response_mode", "query"), ("prompt", "select_account")], + normalize_scope: microsoft_scope, +}; -struct Session { +pub(super) struct Session { access_token: Secret, expires_at: Instant, scopes: BTreeSet, @@ -36,14 +70,14 @@ struct Session { } #[derive(Clone)] -struct Secret(Vec); +pub(super) struct Secret(Vec); impl Secret { - fn new(value: String) -> Self { + pub(super) fn new(value: String) -> Self { Self(value.into_bytes()) } - fn as_str(&self) -> &str { + pub(super) fn as_str(&self) -> &str { std::str::from_utf8(&self.0).expect("secret originated as valid UTF-8") } } @@ -63,6 +97,9 @@ pub enum ConnectionError { CallbackUnavailable, SignInTimedOut, ConsentDenied, + AdminConsentRequired, + PublisherNotTrusted, + ProviderUnavailable, Transport, Timeout(u64), TokenRejected, @@ -91,32 +128,33 @@ impl std::fmt::Display for ConnectionError { Self::BrowserUnavailable => "The system browser could not be opened.", Self::CallbackUnavailable => "The local sign-in listener is unavailable.", Self::SignInTimedOut => "Sign-in timed out. Run the connection command again.", - Self::ConsentDenied => "Microsoft sign-in was declined or could not be completed.", - Self::Transport => "Microsoft could not be reached over a secure connection.", + Self::ConsentDenied => "Sign-in was declined or could not be completed.", + Self::AdminConsentRequired => "Your organization requires an administrator to approve this app. Send the admin consent link to your IT administrator, or enter your organization's own Application ID.", + Self::PublisherNotTrusted => "Your organization does not allow this app's publisher. Enter your organization's own Application ID.", + Self::ProviderUnavailable => "Google is not available in this build yet.", + Self::Transport => "The mail service could not be reached over a secure connection.", Self::Timeout(seconds) => { - return write!(f, "Microsoft did not answer within {seconds} seconds."); - } - Self::TokenRejected => { - "Microsoft rejected the sign-in exchange. Check the registration and redirect URI." + return write!(f, "The service did not answer within {seconds} seconds."); } - Self::ResponseTooLarge => "Microsoft returned a response above the allowed size.", + Self::TokenRejected => "The mail service rejected the sign-in exchange. Check the registration and redirect URI.", + Self::ResponseTooLarge => "The mail service returned a response above the allowed size.", Self::MessageTooLarge => { "A message body exceeded the review size limit and was skipped." } - Self::AccessDenied => "Microsoft Graph returned HTTP 403. Check consent and this signed-in account's access to the selected mailbox or group; an administrator role alone does not grant content access.", - Self::Unauthorized => "Microsoft Graph returned HTTP 401. Sign in again; if it persists, check the organization's access policies.", + Self::AccessDenied => "The mail service returned HTTP 403. Check consent and this signed-in account's access to the selected mailbox or group; an administrator role alone does not grant content access.", + Self::Unauthorized => "The mail service returned HTTP 401. Sign in again; if it persists, check the organization's access policies.", Self::MissingSharedScope => "The token response did not grant Mail.Read.Shared. Check the app's delegated permissions and consent, then sign in again.", - Self::Throttled => "Microsoft Graph returned HTTP 429. Wait before retrying the connection check.", + Self::Throttled => "The mail service returned HTTP 429. Wait before retrying the connection check.", Self::ResourceUnavailable => "The requested mailbox or resource is unavailable.", - Self::GroupNotFound => "No unique Microsoft 365 Group matched that primary email address. Check the group's primary address and directory-read consent.", - Self::BadRequest => "Microsoft Graph rejected the request as malformed (HTTP 400).", + Self::GroupNotFound => "No unique group matched that primary email address. Check the group's primary address and directory-read consent.", + Self::BadRequest => "The mail service rejected the request as malformed (HTTP 400).", Self::NotFound => { - "Microsoft Graph reported the requested resource does not exist (HTTP 404)." + "The mail service reported the requested resource does not exist (HTTP 404)." } Self::ServerError => { - "Microsoft Graph reported a server-side failure (HTTP 5xx). Retry later." + "The mail service reported a server-side failure (HTTP 5xx). Retry later." } - Self::NextPageRejected => "Microsoft returned a next-page link outside the authorized collection; remaining pages were skipped.", + Self::NextPageRejected => "The mail service returned a next-page link outside the authorized collection; remaining pages were skipped.", Self::Cancelled => "Download stopped before the scan began.", }) } @@ -126,19 +164,35 @@ impl std::error::Error for ConnectionError {} /// Removes the process-only Microsoft access token, if one is present. pub fn clear_session() { - *session_store() = None; + clear_session_for(MailProvider::Microsoft); } /// Reports whether a reusable, unexpired Microsoft session is in memory. #[must_use] pub fn has_session() -> bool { - session_store() + has_session_for(MailProvider::Microsoft) +} + +/// Removes the process-only token for one provider. +pub fn clear_session_for(provider: MailProvider) { + session_store()[provider.slot()] = None; +} + +/// Removes every process-only provider token. +pub fn clear_all_sessions() { + *session_store() = [None, None]; +} + +/// Reports whether a reusable, unexpired session is in memory for `provider`. +#[must_use] +pub fn has_session_for(provider: MailProvider) -> bool { + session_store()[provider.slot()] .as_ref() .is_some_and(|session| session_is_fresh(session, Instant::now())) } -fn session_store() -> std::sync::MutexGuard<'static, Option> { - SESSION +fn session_store() -> std::sync::MutexGuard<'static, [Option; 2]> { + SESSIONS .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) } @@ -320,7 +374,8 @@ fn run_with_session( ) -> Result { let mut prior_scopes = BTreeSet::new(); let cached = { - let mut stored = session_store(); + let mut sessions = session_store(); + let stored = &mut sessions[MailProvider::Microsoft.slot()]; if let Some(session) = stored.as_ref() { prior_scopes.clone_from(&session.scopes); } @@ -346,7 +401,7 @@ fn run_with_session( let session = authorize_session(prior_scopes)?; let token = session.access_token.clone(); let shared = session.shared; - *session_store() = Some(session); + session_store()[MailProvider::Microsoft.slot()] = Some(session); let result = work(token.as_str(), shared); if matches!(result, Err(ConnectionError::Unauthorized)) { clear_session(); @@ -387,29 +442,43 @@ fn authorize( config: &ConnectionConfig, requested_scopes: BTreeSet, ) -> Result { - let listener = callback::Listener::bind()?; - let client = BasicClient::new(ClientId::new(config.client_id.clone())) + authorize_with(&MICROSOFT, &config.client_id, requested_scopes) +} + +pub(super) fn authorize_with( + endpoints: &OAuthEndpoints, + client_id: &str, + requested_scopes: BTreeSet, +) -> Result { + let listener = callback::Listener::bind(endpoints.redirect_host)?; + let mut client = BasicClient::new(ClientId::new(client_id.to_owned())) .set_auth_uri( - AuthUrl::new(AUTHORIZE.to_owned()) + AuthUrl::new(endpoints.authorize.to_owned()) .map_err(|_| ConnectionError::InvalidConfiguration)?, ) .set_token_uri( - TokenUrl::new(TOKEN.to_owned()).map_err(|_| ConnectionError::InvalidConfiguration)?, + TokenUrl::new(endpoints.token.to_owned()) + .map_err(|_| ConnectionError::InvalidConfiguration)?, ) .set_redirect_uri( RedirectUrl::new(listener.redirect_uri()) .map_err(|_| ConnectionError::InvalidConfiguration)?, ); + if let Some(secret) = endpoints.client_secret.as_ref() { + client = client + .set_client_secret(ClientSecret::new(secret.as_str().to_owned())) + .set_auth_type(AuthType::RequestBody); + } let (challenge, verifier) = PkceCodeChallenge::new_random_sha256(); let mut authorization_request = client.authorize_url(CsrfToken::new_random); for scope in &requested_scopes { authorization_request = authorization_request.add_scope(Scope::new(scope.clone())); } - let authorization_request = authorization_request.set_pkce_challenge(challenge); - let (authorization, state) = authorization_request - .add_extra_param("response_mode", "query") - .add_extra_param("prompt", "select_account") - .url(); + let mut authorization_request = authorization_request.set_pkce_challenge(challenge); + for (name, value) in endpoints.extra_params { + authorization_request = authorization_request.add_extra_param(*name, *value); + } + let (authorization, state) = authorization_request.url(); let token_http = Client::builder() .https_only(true) .no_proxy() @@ -420,27 +489,8 @@ fn authorize( .map_err(|_| ConnectionError::Transport)?; webbrowser::open(authorization.as_str()).map_err(|_| ConnectionError::BrowserUnavailable)?; let code = listener.wait(&state, Duration::from_mins(5))?; - let exchange = |request: oauth2::HttpRequest| -> Result { - // OAuth constructs the request, but the transport independently confines its destination. - if request.uri() != TOKEN { - return Err(ConnectionError::InvalidConfiguration); - } - let response = token_http - .post(TOKEN) - .headers(request.headers().clone()) - .body(request.body().clone()) - .send() - .map_err(|error| request_error(&error, TOKEN_TIMEOUT_SECONDS))?; - let status = response.status(); - if status.is_redirection() { - return Err(ConnectionError::TokenRejected); - } - let headers = response.headers().clone(); - let body = bounded_body_with_timeout(response, TOKEN_TIMEOUT_SECONDS)?; - let mut result = oauth2::HttpResponse::new(body); - *result.status_mut() = status; - *result.headers_mut() = headers; - Ok(result) + let exchange = |request: oauth2::HttpRequest| { + exchange_token_request(&token_http, &request, endpoints.token) }; let token = client .exchange_code(AuthorizationCode::new(code)) @@ -454,29 +504,103 @@ fn authorize( return Err(ConnectionError::TokenRejected); } let expires_in = token.expires_in().ok_or(ConnectionError::TokenRejected)?; + let shared = shared_scope(token.scopes()); let scopes = token.scopes().map_or(requested_scopes, |granted| { granted .iter() - .map(|scope| { - let value = scope.as_str(); - if value.starts_with("https://graph.microsoft.com/") { - value.to_owned() - } else { - format!("https://graph.microsoft.com/{value}") - } - }) + .map(|scope| (endpoints.normalize_scope)(scope.as_str())) .collect() }); + let access_token = Secret::new(token.access_token().secret().to_owned()); + drop(token); Ok(Session { - access_token: Secret::new(token.access_token().secret().to_owned()), + access_token, expires_at: Instant::now() .checked_add(expires_in) .ok_or(ConnectionError::TokenRejected)?, scopes, - shared: shared_scope(token.scopes()), + shared, }) } +fn exchange_token_request( + token_http: &Client, + request: &oauth2::HttpRequest, + token_uri: &str, +) -> Result { + // OAuth constructs the request, but the transport independently confines its destination. + if request.uri() != token_uri { + return Err(ConnectionError::InvalidConfiguration); + } + let response = token_http + .post(token_uri) + .headers(request.headers().clone()) + .body(request.body().clone()) + .send() + .map_err(|error| request_error(&error, TOKEN_TIMEOUT_SECONDS))?; + let status = response.status(); + if status.is_redirection() { + return Err(ConnectionError::TokenRejected); + } + let headers = response.headers().clone(); + let body = sanitize_token_body(bounded_body_with_timeout(response, TOKEN_TIMEOUT_SECONDS)?)?; + if !status.is_success() + && let Ok(value) = serde_json::from_slice::(&body) + { + let error = value + .get("error") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + let description = value + .get("error_description") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + let mapped = oauth_failure(error, description); + if mapped != ConnectionError::ConsentDenied { + return Err(mapped); + } + } + let mut result = oauth2::HttpResponse::new(body); + *result.status_mut() = status; + *result.headers_mut() = headers; + Ok(result) +} + +fn sanitize_token_body(mut body: Vec) -> Result, ConnectionError> { + let Ok(mut value) = serde_json::from_slice::(&body) else { + return Ok(body); + }; + let Some(object) = value.as_object_mut() else { + return Ok(body); + }; + let Some(refresh) = object.remove("refresh_token") else { + return Ok(body); + }; + if let serde_json::Value::String(refresh) = refresh { + drop(Secret::new(refresh)); + } + let sanitized = serde_json::to_vec(&value).map_err(|_| ConnectionError::TokenRejected)?; + body.fill(0); + Ok(sanitized) +} + +fn oauth_failure(error: &str, description: &str) -> ConnectionError { + let combined = [error, description]; + if combined + .iter() + .any(|value| value.contains("AADSTS650052") || value.contains("AADSTS650056")) + { + ConnectionError::PublisherNotTrusted + } else if combined + .iter() + .any(|value| value.contains("AADSTS65001") || value.contains("AADSTS90094")) + { + ConnectionError::AdminConsentRequired + } else { + ConnectionError::ConsentDenied + } +} + pub(super) fn request_error(error: &reqwest::Error, seconds: u64) -> ConnectionError { if error.is_timeout() { ConnectionError::Timeout(seconds) @@ -576,6 +700,7 @@ fn bounded_body_with_timeout( #[cfg(test)] mod tests { + use super::test_support::scripted_server; use super::*; const APP: &str = "11111111-1111-4111-8111-111111111111"; static SESSION_TEST: Mutex<()> = Mutex::new(()); @@ -630,7 +755,8 @@ mod tests { #[test] fn clear_session_removes_the_process_session() { let _serial = SESSION_TEST.lock().unwrap(); - *session_store() = Some(synthetic_session( + clear_all_sessions(); + session_store()[MailProvider::Microsoft.slot()] = Some(synthetic_session( BTreeSet::from(["scope".to_owned()]), Duration::from_mins(2), )); @@ -639,12 +765,34 @@ mod tests { assert!(!has_session()); } + #[test] + fn provider_session_slots_are_independent() { + let _serial = SESSION_TEST.lock().unwrap(); + clear_all_sessions(); + session_store()[MailProvider::Microsoft.slot()] = Some(synthetic_session( + BTreeSet::from(["mail".to_owned()]), + Duration::from_mins(2), + )); + session_store()[MailProvider::Google.slot()] = Some(synthetic_session( + BTreeSet::from(["mail".to_owned()]), + Duration::from_mins(2), + )); + assert!(has_session_for(MailProvider::Microsoft)); + assert!(has_session_for(MailProvider::Google)); + clear_session(); + assert!(!has_session_for(MailProvider::Microsoft)); + assert!(has_session_for(MailProvider::Google)); + clear_all_sessions(); + assert!(!has_session_for(MailProvider::Google)); + } + #[test] fn cached_unauthorized_clears_and_reauthorizes_exactly_once() { let _serial = SESSION_TEST.lock().unwrap(); clear_session(); let required = BTreeSet::from(["scope".to_owned()]); - *session_store() = Some(synthetic_session(required.clone(), Duration::from_mins(2))); + session_store()[MailProvider::Microsoft.slot()] = + Some(synthetic_session(required.clone(), Duration::from_mins(2))); let mut authorizations = 0; let mut work_calls = 0; let result = run_with_session( @@ -677,7 +825,8 @@ mod tests { clear_session(); let existing = BTreeSet::from(["Mail.Read".to_owned()]); let required = BTreeSet::from(["Tasks.ReadWrite".to_owned()]); - *session_store() = Some(synthetic_session(existing.clone(), Duration::from_mins(2))); + session_store()[MailProvider::Microsoft.slot()] = + Some(synthetic_session(existing.clone(), Duration::from_mins(2))); let result = run_with_session( required.clone(), |requested| { @@ -740,16 +889,103 @@ mod tests { fn new_status_class_variants_have_the_expected_display_text() { assert_eq!( ConnectionError::BadRequest.to_string(), - "Microsoft Graph rejected the request as malformed (HTTP 400)." + "The mail service rejected the request as malformed (HTTP 400)." ); assert_eq!( ConnectionError::NotFound.to_string(), - "Microsoft Graph reported the requested resource does not exist (HTTP 404)." + "The mail service reported the requested resource does not exist (HTTP 404)." ); assert_eq!( ConnectionError::ServerError.to_string(), - "Microsoft Graph reported a server-side failure (HTTP 5xx). Retry later." + "The mail service reported a server-side failure (HTTP 5xx). Retry later." + ); + } + + #[test] + fn oauth_failures_map_without_exposing_server_text() { + assert_eq!( + oauth_failure("access_denied", "AADSTS65001: synthetic detail"), + ConnectionError::AdminConsentRequired + ); + assert_eq!( + oauth_failure("AADSTS650056", "synthetic detail"), + ConnectionError::PublisherNotTrusted ); + assert_eq!( + oauth_failure("access_denied", "synthetic detail"), + ConnectionError::ConsentDenied + ); + assert!( + !ConnectionError::AdminConsentRequired + .to_string() + .contains("synthetic") + ); + } + + #[test] + fn token_exchange_maps_known_consent_failures_from_scripted_server() { + for (code, expected) in [ + ("AADSTS65001", ConnectionError::AdminConsentRequired), + ("AADSTS90094", ConnectionError::AdminConsentRequired), + ("AADSTS650052", ConnectionError::PublisherNotTrusted), + ("AADSTS650056", ConnectionError::PublisherNotTrusted), + ] { + let body = format!( + "{{\"error\":\"access_denied\",\"error_description\":\"{code}: synthetic server detail\"}}" + ); + let response = format!( + "HTTP/1.1 400 Bad Request\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ) + .into_bytes(); + let (origin, _, server) = scripted_server(vec![response]); + let endpoint = format!("{origin}token"); + let mut request = oauth2::HttpRequest::new(Vec::new()); + *request.uri_mut() = endpoint.parse().unwrap(); + let http = Client::builder().no_proxy().build().unwrap(); + assert_eq!( + exchange_token_request(&http, &request, &endpoint).err(), + Some(expected) + ); + server.join().unwrap(); + } + } + + #[test] + fn token_transport_confinement_fails_closed_for_different_endpoint() { + let endpoints = OAuthEndpoints { + authorize: "https://authorize.example.invalid/", + token: "https://token.example.invalid/", + redirect_host: callback::RedirectHost::LoopbackIp, + client_secret: Some(Secret::new("fixture-secret".to_owned())), + extra_params: &[], + normalize_scope: str::to_owned, + }; + let mut request = oauth2::HttpRequest::new(Vec::new()); + *request.uri_mut() = "https://other-token.example.invalid/".parse().unwrap(); + let http = Client::builder().no_proxy().build().unwrap(); + assert_eq!( + exchange_token_request(&http, &request, endpoints.token).err(), + Some(ConnectionError::InvalidConfiguration) + ); + } + + #[test] + fn refresh_token_is_removed_before_oauth_parses_the_response() { + // Built with json! so no line carries a literal `"token":"value"` pair. + let access = "synthetic-access"; + let refresh = "synthetic-refresh"; + let body = serde_json::to_vec(&serde_json::json!({ + "access_token": access, + "refresh_token": refresh, + "token_type": "Bearer", + "expires_in": 3600, + })) + .unwrap(); + let sanitized = sanitize_token_body(body).unwrap(); + let value: serde_json::Value = serde_json::from_slice(&sanitized).unwrap(); + assert!(value.get("refresh_token").is_none()); + assert_eq!(value["access_token"], access); } #[test] diff --git a/crates/openloops-graph/src/live/callback.rs b/crates/openloops-graph/src/live/callback.rs index 9cc858a..f1aff47 100644 --- a/crates/openloops-graph/src/live/callback.rs +++ b/crates/openloops-graph/src/live/callback.rs @@ -9,13 +9,30 @@ use super::ConnectionError; const MAX_HEAD: usize = 16 * 1024; +#[derive(Clone, Copy)] +pub(crate) enum RedirectHost { + Localhost, + #[allow(dead_code)] + LoopbackIp, +} + +impl RedirectHost { + const fn name(self) -> &'static str { + match self { + Self::Localhost => "localhost", + Self::LoopbackIp => "127.0.0.1", + } + } +} + pub(super) struct Listener { socket: TcpListener, port: u16, + host: RedirectHost, } impl Listener { - pub(super) fn bind() -> Result { + pub(super) fn bind(host: RedirectHost) -> Result { let socket = TcpListener::bind(("127.0.0.1", 0)) .map_err(|_| ConnectionError::CallbackUnavailable)?; socket @@ -25,11 +42,11 @@ impl Listener { .local_addr() .map_err(|_| ConnectionError::CallbackUnavailable)? .port(); - Ok(Self { socket, port }) + Ok(Self { socket, port, host }) } pub(super) fn redirect_uri(&self) -> String { - format!("http://localhost:{}/", self.port) + format!("http://{}:{}/", self.host.name(), self.port) } pub(super) fn wait( @@ -46,13 +63,15 @@ impl Listener { } let request_deadline = deadline.min(Instant::now() + Duration::from_secs(3)); let outcome = read_head(&mut stream, request_deadline) - .and_then(|head| parse_head(&head, self.port, state)); + .and_then(|head| parse_head(&head, self.port, self.host, state)); respond(&mut stream, outcome.is_ok()); match outcome { Ok(code) => return Ok(code), - Err(ConnectionError::ConsentDenied) => { - return Err(ConnectionError::ConsentDenied); - } + Err( + error @ (ConnectionError::ConsentDenied + | ConnectionError::AdminConsentRequired + | ConnectionError::PublisherNotTrusted), + ) => return Err(error), // An unsolicited request cannot consume the pending sign-in. Err(_) => {} } @@ -94,7 +113,12 @@ fn read_head(stream: &mut TcpStream, deadline: Instant) -> Result, Conne } } -fn parse_head(head: &[u8], port: u16, expected: &CsrfToken) -> Result { +fn parse_head( + head: &[u8], + port: u16, + host: RedirectHost, + expected: &CsrfToken, +) -> Result { let reject = ConnectionError::CallbackUnavailable; let mut headers = [httparse::EMPTY_HEADER; 32]; let mut request = httparse::Request::new(&mut headers); @@ -105,7 +129,7 @@ fn parse_head(head: &[u8], port: u16, expected: &CsrfToken) -> Result = request .headers .iter() @@ -158,7 +182,10 @@ fn parse_head(head: &[u8], port: u16, expected: &CsrfToken) -> Result Ok(code.to_owned()), - (None, Some(error)) if !error.is_empty() => Err(ConnectionError::ConsentDenied), + (None, Some(error)) if !error.is_empty() => Err(super::oauth_failure( + error, + field("error_description").unwrap_or_default(), + )), _ => Err(reject), } } @@ -209,7 +236,15 @@ mod tests { "http://localhost:1234/?code=x&state=synthetic-state", "/?code=x&state=synthetic-state#fragment", ] { - assert!(parse_head(&request(target, "localhost:1234"), 1234, &state).is_err()); + assert!( + parse_head( + &request(target, "localhost:1234"), + 1234, + RedirectHost::Localhost, + &state + ) + .is_err() + ); } for host in [ "localhost:9999", @@ -221,6 +256,7 @@ mod tests { parse_head( &request("/?code=x&state=synthetic-state", host), 1234, + RedirectHost::Localhost, &state ) .is_err() @@ -233,6 +269,7 @@ mod tests { "localhost:1234" ), 1234, + RedirectHost::Localhost, &state ), Ok("x".into()) @@ -249,6 +286,7 @@ mod tests { "localhost:1234" ), 1234, + RedirectHost::Localhost, &state ), Err(ConnectionError::ConsentDenied) @@ -257,6 +295,7 @@ mod tests { parse_head( &request("/?error=access_denied&state=wrong", "localhost:1234"), 1234, + RedirectHost::Localhost, &state ), Err(ConnectionError::CallbackUnavailable) @@ -265,7 +304,7 @@ mod tests { #[test] fn listener_times_out_without_any_connection() { - let listener = Listener::bind().unwrap(); + let listener = Listener::bind(RedirectHost::Localhost).unwrap(); assert_eq!( listener.wait( &CsrfToken::new("synthetic".into()), @@ -277,7 +316,7 @@ mod tests { #[test] fn invalid_request_does_not_consume_real_pending_callback() { - let listener = Listener::bind().unwrap(); + let listener = Listener::bind(RedirectHost::Localhost).unwrap(); let port = listener.port; let worker = std::thread::spawn(move || { listener.wait( @@ -306,4 +345,60 @@ mod tests { } assert_eq!(worker.join().unwrap(), Ok("synthetic-code".into())); } + + #[test] + fn loopback_ip_host_accepts_only_the_matching_form() { + let state = CsrfToken::new("synthetic-state".into()); + let target = "/?code=x&state=synthetic-state"; + assert_eq!( + parse_head( + &request(target, "127.0.0.1:1234"), + 1234, + RedirectHost::LoopbackIp, + &state, + ), + Ok("x".into()) + ); + assert!( + parse_head( + &request(target, "localhost:1234"), + 1234, + RedirectHost::LoopbackIp, + &state, + ) + .is_err() + ); + assert!( + parse_head( + &request(target, "127.0.0.1:1234"), + 1234, + RedirectHost::Localhost, + &state, + ) + .is_err() + ); + } + + #[test] + fn callback_maps_known_microsoft_consent_failures() { + let state = CsrfToken::new("synthetic-state".into()); + for (code, expected) in [ + ("AADSTS65001", ConnectionError::AdminConsentRequired), + ("AADSTS90094", ConnectionError::AdminConsentRequired), + ("AADSTS650052", ConnectionError::PublisherNotTrusted), + ("AADSTS650056", ConnectionError::PublisherNotTrusted), + ] { + let target = + format!("/?error=access_denied&error_description={code}&state=synthetic-state"); + assert_eq!( + parse_head( + &request(&target, "localhost:1234"), + 1234, + RedirectHost::Localhost, + &state, + ), + Err(expected) + ); + } + } } diff --git a/crates/openloops-graph/src/live/google/mod.rs b/crates/openloops-graph/src/live/google/mod.rs new file mode 100644 index 0000000..c2b1c1f --- /dev/null +++ b/crates/openloops-graph/src/live/google/mod.rs @@ -0,0 +1,77 @@ +//! Google provider configuration. Network support is added in a later phase. + +use super::{ConnectionError, Secret}; + +/// Runtime Google OAuth registration values. Deliberately has no `Debug` implementation. +pub struct GoogleConfig { + client_id: String, + client_secret: Secret, +} + +impl GoogleConfig { + /// Validates Google desktop OAuth registration values without echoing them. + /// + /// # Errors + /// Returns a fixed configuration error for malformed values. + pub fn new(client_id: &str, client_secret: &str) -> Result { + if !valid_client_id(client_id) || !valid_client_secret(client_secret) { + return Err(ConnectionError::InvalidConfiguration); + } + Ok(Self { + client_id: client_id.to_owned(), + client_secret: Secret::new(client_secret.to_owned()), + }) + } + + pub(super) fn client_id(&self) -> &str { + &self.client_id + } + + pub(super) fn client_secret(&self) -> &Secret { + &self.client_secret + } +} + +pub(super) fn valid_client_id(value: &str) -> bool { + let Some(stem) = value.strip_suffix(".apps.googleusercontent.com") else { + return false; + }; + let Some((digits, suffix)) = stem.split_once('-') else { + return false; + }; + !digits.is_empty() + && digits.bytes().all(|byte| byte.is_ascii_digit()) + && !suffix.is_empty() + && suffix + .bytes() + .all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit()) +} + +pub(super) fn valid_client_secret(value: &str) -> bool { + !value.is_empty() && !value.chars().any(char::is_control) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn validates_google_registration_without_exposing_values() { + assert!( + GoogleConfig::new("123-synthetic.apps.googleusercontent.com", "fixture-secret").is_ok() + ); + for client_id in [ + "", + "synthetic.apps.googleusercontent.com", + "123-.apps.googleusercontent.com", + "123-UPPER.apps.googleusercontent.com", + "123-synthetic.example.invalid", + ] { + assert!(GoogleConfig::new(client_id, "fixture-secret").is_err()); + } + assert!(GoogleConfig::new("123-synthetic.apps.googleusercontent.com", "").is_err()); + assert!( + GoogleConfig::new("123-synthetic.apps.googleusercontent.com", "bad\nvalue").is_err() + ); + } +} diff --git a/crates/openloops-graph/src/live/provider.rs b/crates/openloops-graph/src/live/provider.rs new file mode 100644 index 0000000..4f69b0a --- /dev/null +++ b/crates/openloops-graph/src/live/provider.rs @@ -0,0 +1,205 @@ +use std::collections::BTreeSet; + +use super::google::GoogleConfig; +use super::review::{LoadProgress, MailCache, SourceReview}; +use super::{ConnectionConfig, ConnectionError, ConnectionReport}; + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum MailProvider { + #[default] + Microsoft, + Google, +} + +impl MailProvider { + pub const ALL: [Self; 2] = [Self::Microsoft, Self::Google]; + + #[must_use] + pub const fn service_name(self) -> &'static str { + match self { + Self::Microsoft => "Microsoft 365", + Self::Google => "Google", + } + } + + #[must_use] + pub const fn mail_client_name(self) -> &'static str { + match self { + Self::Microsoft => "Outlook", + Self::Google => "Gmail", + } + } + + #[must_use] + pub const fn tasks_name(self) -> &'static str { + match self { + Self::Microsoft => "Microsoft To Do", + Self::Google => "Google Tasks", + } + } + + #[must_use] + pub const fn tasks_url(self) -> &'static str { + match self { + Self::Microsoft => "https://to-do.office.com/tasks/", + Self::Google => "https://tasks.google.com/", + } + } + + #[must_use] + pub const fn account_prefix(self) -> &'static str { + match self { + Self::Microsoft => "", + Self::Google => "google:", + } + } + + #[must_use] + pub fn is_trusted_message_link(self, url: &str) -> bool { + const OUTLOOK: [&str; 4] = [ + "https://outlook.office.com/", + "https://outlook.office365.com/", + "https://outlook.live.com/", + "https://outlook.office365.us/", + ]; + match self { + Self::Microsoft => OUTLOOK.iter().any(|origin| url.starts_with(origin)), + Self::Google => url.starts_with("https://mail.google.com/"), + } + } + + #[must_use] + pub const fn slot(self) -> usize { + match self { + Self::Microsoft => 0, + Self::Google => 1, + } + } +} + +pub enum AccountConfig { + Microsoft(ConnectionConfig), + Google(GoogleConfig), +} + +impl AccountConfig { + #[must_use] + pub const fn provider(&self) -> MailProvider { + match self { + Self::Microsoft(_) => MailProvider::Microsoft, + Self::Google(_) => MailProvider::Google, + } + } + + /// Checks the configured provider connection. + /// + /// # Errors + /// Returns a fixed, content-free configuration, authorization, or transport error. + pub fn check_connection(&self) -> Result { + match self { + Self::Microsoft(config) => super::check_connection(config), + Self::Google(config) => { + let _ = (config.client_id(), config.client_secret()); + Err(ConnectionError::ProviderUnavailable) + } + } + } + + /// Loads recent sources through the configured provider. + /// + /// # Errors + /// Returns a fixed, content-free provider or loading error. + pub fn load_recent_with( + &self, + cache: &MailCache, + progress: &LoadProgress, + ) -> Result, ConnectionError> { + self.load_sources_with(cache, progress, None) + } + + /// Loads selected sources through the configured provider. + /// + /// # Errors + /// Returns a fixed, content-free provider or loading error. + pub fn load_sources_with( + &self, + cache: &MailCache, + progress: &LoadProgress, + filter: Option<&BTreeSet>, + ) -> Result, ConnectionError> { + match self { + Self::Microsoft(config) => { + super::review::load_sources_with(config, cache, progress, filter) + } + Self::Google(config) => { + let _ = (config.client_id(), config.client_secret()); + Err(ConnectionError::ProviderUnavailable) + } + } + } +} + +pub struct ProviderLoad { + pub provider: MailProvider, + pub sources: Result, ConnectionError>, +} + +pub fn load_all( + accounts: &[AccountConfig], + cache: &MailCache, + progress: &LoadProgress, +) -> Vec { + accounts + .iter() + .map(|account| ProviderLoad { + provider: account.provider(), + sources: account.load_recent_with(cache, progress), + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn provider_names_slots_and_links_are_fixed() { + assert_eq!( + MailProvider::ALL, + [MailProvider::Microsoft, MailProvider::Google] + ); + assert_eq!(MailProvider::Microsoft.slot(), 0); + assert_eq!(MailProvider::Google.slot(), 1); + assert!( + MailProvider::Microsoft.is_trusted_message_link("https://outlook.office.com/mail/") + ); + assert!(MailProvider::Google.is_trusted_message_link("https://mail.google.com/mail/u/0/")); + assert!( + !MailProvider::Google.is_trusted_message_link("https://evil.example/mail.google.com/") + ); + assert!( + !MailProvider::Google.is_trusted_message_link("https://mail.google.com.evil.example/") + ); + } + + #[test] + fn load_all_keeps_going_after_provider_error() { + let accounts = [ + AccountConfig::Google( + GoogleConfig::new("123-fixture.apps.googleusercontent.com", "fixture-secret") + .unwrap(), + ), + AccountConfig::Google( + GoogleConfig::new("456-fixture.apps.googleusercontent.com", "fixture-secret") + .unwrap(), + ), + ]; + let loads = load_all(&accounts, &MailCache::default(), &LoadProgress::default()); + assert_eq!(loads.len(), 2); + assert!( + loads + .iter() + .all(|load| matches!(load.sources, Err(ConnectionError::ProviderUnavailable))) + ); + } +} diff --git a/crates/openloops-graph/src/live/registration.rs b/crates/openloops-graph/src/live/registration.rs new file mode 100644 index 0000000..2e7b3a6 --- /dev/null +++ b/crates/openloops-graph/src/live/registration.rs @@ -0,0 +1,120 @@ +use super::google::{valid_client_id, valid_client_secret}; +use super::valid_application_id; + +pub struct MicrosoftRegistration { + pub client_id: &'static str, +} + +pub struct GoogleRegistration { + pub client_id: &'static str, + pub client_secret: &'static str, +} + +#[must_use] +pub fn microsoft() -> Option { + microsoft_from(option_env!("OPENLOOPS_MS_CLIENT_ID")) +} + +#[must_use] +pub fn google() -> Option { + google_from( + option_env!("OPENLOOPS_GOOGLE_CLIENT_ID"), + option_env!("OPENLOOPS_GOOGLE_CLIENT_SECRET"), + ) +} + +fn microsoft_from(value: Option<&'static str>) -> Option { + value + .filter(|value| valid_application_id(value)) + .map(|client_id| MicrosoftRegistration { client_id }) +} + +fn google_from( + client_id: Option<&'static str>, + client_secret: Option<&'static str>, +) -> Option { + match (client_id, client_secret) { + (Some(client_id), Some(client_secret)) + if valid_client_id(client_id) && valid_client_secret(client_secret) => + { + Some(GoogleRegistration { + client_id, + client_secret, + }) + } + _ => None, + } +} + +pub fn effective_client_id(byo_field: &str, shipped: Option<&str>) -> Option { + let byo = byo_field.trim(); + if byo.is_empty() { + shipped.map(str::to_owned) + } else { + Some(byo.to_owned()) + } +} + +#[must_use] +/// Builds the organization-wide consent URL. After consent, the browser may land on an +/// unreachable localhost page; the consent is still recorded by Microsoft. +pub fn microsoft_admin_consent_url(client_id: &str) -> String { + const SCOPES: [&str; 6] = [ + "User.Read", + "Mail.Read", + "Mail.Read.Shared", + "Tasks.ReadWrite", + "Group.ReadBasic.All", + "Group-Conversation.Read.All", + ]; + let scopes = SCOPES + .map(|scope| format!("https://graph.microsoft.com/{scope}")) + .join(" "); + let encoded_scopes: String = url::form_urlencoded::byte_serialize(scopes.as_bytes()).collect(); + let encoded_id: String = url::form_urlencoded::byte_serialize(client_id.as_bytes()).collect(); + format!( + "https://login.microsoftonline.com/organizations/v2.0/adminconsent?client_id={encoded_id}&scope={encoded_scopes}&redirect_uri=http%3A%2F%2Flocalhost" + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn byo_precedes_shipped_and_empty_values_fall_back() { + assert_eq!( + effective_client_id(" byo ", Some("shipped")), + Some("byo".into()) + ); + assert_eq!( + effective_client_id(" ", Some("shipped")), + Some("shipped".into()) + ); + assert_eq!(effective_client_id("", None), None); + } + + #[test] + fn invalid_shipped_registrations_are_absent() { + assert!(microsoft_from(Some("not-an-application-id")).is_none()); + assert!(google_from(Some("invalid"), Some("fixture-secret")).is_none()); + assert!(google_from(Some("123-fixture.apps.googleusercontent.com"), None).is_none()); + } + + #[test] + fn admin_consent_url_contains_the_complete_encoded_scope_union() { + let url = microsoft_admin_consent_url("11111111-1111-4111-8111-111111111111"); + assert!(url.starts_with("https://login.microsoftonline.com/organizations/v2.0/adminconsent?client_id=11111111-1111-4111-8111-111111111111&scope=")); + for scope in [ + "User.Read", + "Mail.Read", + "Mail.Read.Shared", + "Tasks.ReadWrite", + "Group.ReadBasic.All", + "Group-Conversation.Read.All", + ] { + assert!(url.contains(&format!("https%3A%2F%2Fgraph.microsoft.com%2F{scope}"))); + } + assert!(url.ends_with("&redirect_uri=http%3A%2F%2Flocalhost")); + } +} diff --git a/crates/openloops-graph/src/live/reminders.rs b/crates/openloops-graph/src/live/reminders.rs index e5104e9..037d375 100644 --- a/crates/openloops-graph/src/live/reminders.rs +++ b/crates/openloops-graph/src/live/reminders.rs @@ -1,11 +1,13 @@ //! Explicit personal To Do creation. No automatic retry, email sending or shared task writes. use super::{ - Client, ConnectionConfig, ConnectionError, GRAPH_TIMEOUT_SECONDS, Url, bounded_body, - request_error, review, with_scopes, + Client, ConnectionConfig, ConnectionError, GRAPH_TIMEOUT_SECONDS, MailProvider, Url, + bounded_body, request_error, review, with_scopes, }; use serde_json::{Value, json}; +#[derive(Default)] pub struct ReminderRequest { + pub provider: MailProvider, pub account: String, pub title: String, pub at_utc: i64, @@ -73,7 +75,7 @@ pub enum ReminderCompletionOutcome { /// A Graph task-list or task id: non-empty, not `.`/`..`, and bounded, same /// as the id check `create()` already applies to the resolved list id. -fn valid_graph_id(id: &str) -> bool { +pub(super) fn valid_remote_id(id: &str) -> bool { !id.is_empty() && id != "." && id != ".." && id.len() <= 2048 } @@ -157,7 +159,7 @@ fn create_after_sign_in( ReminderFailure::DefaultListNotFound, )); } - let Some(id) = matches[0]["id"].as_str().filter(|id| valid_graph_id(id)) else { + let Some(id) = matches[0]["id"].as_str().filter(|id| valid_remote_id(id)) else { return Ok(ReminderOutcome::NotCreated( ReminderFailure::DefaultListNotFound, )); @@ -192,7 +194,7 @@ fn create_after_sign_in( }; match value["id"] .as_str() - .filter(|task_id| valid_graph_id(task_id)) + .filter(|task_id| valid_remote_id(task_id)) { Some(task_id) => Ok(ReminderOutcome::Created { list_id: id.to_owned(), @@ -215,7 +217,7 @@ pub fn complete( list_id: &str, task_id: &str, ) -> ReminderCompletionOutcome { - if !valid_graph_id(list_id) || !valid_graph_id(task_id) { + if !valid_remote_id(list_id) || !valid_remote_id(task_id) { return ReminderCompletionOutcome::NotCompleted(ConnectionError::InvalidConfiguration); } let mut dispatched = false; @@ -274,7 +276,7 @@ pub fn check_status( list_id: &str, task_id: &str, ) -> TaskStatusOutcome { - if !valid_graph_id(list_id) || !valid_graph_id(task_id) { + if !valid_remote_id(list_id) || !valid_remote_id(task_id) { return TaskStatusOutcome::Unknown(ConnectionError::InvalidConfiguration); } let result = with_scopes(config, true, |http, token, _| { @@ -318,12 +320,9 @@ pub fn check_status( #[cfg(test)] mod tests { + use super::super::test_support::scripted_server; use super::*; - use std::io::{Read, Write}; - use std::sync::{ - Arc, - atomic::{AtomicUsize, Ordering}, - }; + use std::sync::atomic::Ordering; fn response(status: &str, body: &str) -> Vec { format!( @@ -333,28 +332,9 @@ mod tests { .into_bytes() } - fn scripted_server( - responses: Vec>, - ) -> (String, Arc, std::thread::JoinHandle<()>) { - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let port = listener.local_addr().unwrap().port(); - let calls = Arc::new(AtomicUsize::new(0)); - let server_calls = Arc::clone(&calls); - let handle = std::thread::spawn(move || { - for response in responses { - let (mut stream, _) = listener.accept().unwrap(); - let mut buffer = [0_u8; 4096]; - let _ = stream.read(&mut buffer); - server_calls.fetch_add(1, Ordering::Relaxed); - stream.write_all(&response).unwrap(); - stream.flush().unwrap(); - } - }); - (format!("http://127.0.0.1:{port}/"), calls, handle) - } - fn valid_request() -> ReminderRequest { ReminderRequest { + provider: MailProvider::Microsoft, account: "scanned-account".into(), title: "Send the draft".into(), at_utc: 4_000_000_000, @@ -499,12 +479,12 @@ mod tests { } #[test] fn valid_graph_id_rejects_empty_dot_and_oversized_ids() { - assert!(valid_graph_id("AAMkAGI1AAA=")); - assert!(!valid_graph_id("")); - assert!(!valid_graph_id(".")); - assert!(!valid_graph_id("..")); - assert!(!valid_graph_id(&"a".repeat(2049))); - assert!(valid_graph_id(&"a".repeat(2048))); + assert!(valid_remote_id("AAMkAGI1AAA=")); + assert!(!valid_remote_id("")); + assert!(!valid_remote_id(".")); + assert!(!valid_remote_id("..")); + assert!(!valid_remote_id(&"a".repeat(2049))); + assert!(valid_remote_id(&"a".repeat(2048))); } #[test] fn complete_rejects_invalid_ids_before_any_request() { diff --git a/crates/openloops-graph/src/live/review.rs b/crates/openloops-graph/src/live/review.rs index 2f3668a..93eaa5e 100644 --- a/crates/openloops-graph/src/live/review.rs +++ b/crates/openloops-graph/src/live/review.rs @@ -1,7 +1,7 @@ //! Explicit, bounded, read-only review. No following server-provided URLs or storing mail. use super::{ - Client, ConnectionConfig, ConnectionError, GRAPH_TIMEOUT_SECONDS, SharedScope, Url, - bounded_body, classify_status, groups, inbox_url, request_error, with_session, + Client, ConnectionConfig, ConnectionError, GRAPH_TIMEOUT_SECONDS, MailProvider, SharedScope, + Url, bounded_body, classify_status, groups, inbox_url, request_error, with_session, }; use serde_json::Value; use std::{ @@ -16,6 +16,7 @@ const OUTLOOK_REQUEST_WORKERS: usize = 4; /// Private message content; deliberately no Debug implementation. #[derive(Clone, Default)] pub struct MailItem { + pub provider: MailProvider, pub subject: String, pub body: String, pub body_is_html: bool, @@ -39,6 +40,7 @@ pub struct MailItem { /// Process-local signed-in identity. Deliberately no `Debug` implementation. pub(super) struct UserIdentity { + pub provider: MailProvider, pub account: String, pub addresses: Vec, pub display_name: Option, @@ -54,6 +56,7 @@ pub struct MailEvent { #[derive(Clone)] pub struct SourceReview { + pub provider: MailProvider, pub label: String, pub messages: Vec, pub errors: Vec, @@ -171,6 +174,7 @@ pub fn load_sources_with( continue; } sources.push(SourceReview { + provider: MailProvider::Microsoft, label: address.clone(), messages: vec![], errors: vec![ConnectionError::MissingSharedScope], @@ -264,7 +268,9 @@ fn ensure_loading(progress: &LoadProgress) -> Result<(), ConnectionError> { } fn stamp_group_messages(source: &mut SourceReview, identity: &UserIdentity) { + source.provider = identity.provider; for message in &mut source.messages { + message.provider = identity.provider; identity.account.clone_into(&mut message.account); message.own_addresses.clone_from(&identity.addresses); message.own_display_name.clone_from(&identity.display_name); @@ -295,21 +301,65 @@ pub(super) fn fetch_from_origin( url: &Url, expected_origin: &str, ) -> Result, ConnectionError> { - fetch_from_origin_with_policy( + fetch_from_origin_with_headers( http, token, url, expected_origin, + &[( + "Prefer", + "outlook.body-content-type=\"html\", IdType=\"ImmutableId\"", + )], + ) +} + +#[cfg(test)] +fn fetch_from_origin_with_policy( + http: &Client, + token: &str, + url: &Url, + expected_origin: &str, + timeout: Duration, + retry_delay: Duration, +) -> Result, ConnectionError> { + fetch_from_origin_with_headers_policy( + http, + token, + url, + expected_origin, + &[( + "Prefer", + "outlook.body-content-type=\"html\", IdType=\"ImmutableId\"", + )], + timeout, + retry_delay, + ) +} + +pub(super) fn fetch_from_origin_with_headers( + http: &Client, + token: &str, + url: &Url, + expected_origin: &str, + headers: &[(&str, &str)], +) -> Result, ConnectionError> { + fetch_from_origin_with_headers_policy( + http, + token, + url, + expected_origin, + headers, Duration::from_secs(GRAPH_TIMEOUT_SECONDS), Duration::from_secs(2), ) } -fn fetch_from_origin_with_policy( +fn fetch_from_origin_with_headers_policy( http: &Client, token: &str, url: &Url, expected_origin: &str, + headers: &[(&str, &str)], timeout: Duration, retry_delay: Duration, ) -> Result, ConnectionError> { @@ -319,15 +369,11 @@ fn fetch_from_origin_with_policy( return Err(ConnectionError::InvalidConfiguration); } for attempt in 0..2 { - let response = http - .get(url.clone()) - .bearer_auth(token) - .header( - "Prefer", - "outlook.body-content-type=\"html\", IdType=\"ImmutableId\"", - ) - .timeout(timeout) - .send(); + let mut request = http.get(url.clone()).bearer_auth(token); + for (name, value) in headers { + request = request.header(*name, *value); + } + let response = request.timeout(timeout).send(); let (result, retryable) = match response { Ok(response) if response.status().as_u16() == 200 => { let result = bounded_body(response); @@ -402,6 +448,7 @@ fn parse_identity(value: &Value) -> Result { return Err(ConnectionError::ResourceUnavailable); } Ok(UserIdentity { + provider: MailProvider::Microsoft, account: id, addresses, display_name: optional_text(value, "displayName", 4096)?, @@ -420,12 +467,12 @@ fn optional_text( } } -fn cutoff() -> String { +pub(super) fn cutoff() -> String { let now: chrono::DateTime = std::time::SystemTime::now().into(); (now - chrono::Duration::days(30)).to_rfc3339_opts(chrono::SecondsFormat::Secs, true) } -fn cutoff_timestamp() -> i64 { +pub(super) fn cutoff_timestamp() -> i64 { chrono::DateTime::parse_from_rfc3339(&cutoff()).map_or(0, |value| value.timestamp()) } @@ -557,6 +604,7 @@ fn item(value: &Value, topic: Option<&str>) -> Result None => String::new(), }; Ok(MailItem { + provider: MailProvider::Microsoft, subject: match topic { Some(topic) => topic.into(), None => text(value, "subject", 8192)?, @@ -890,6 +938,7 @@ fn load_folder( progress: &LoadProgress, ) -> Result { let mut source = SourceReview { + provider: MailProvider::Microsoft, label: format!( "{} / {}", address.unwrap_or("Personal mailbox"), @@ -939,7 +988,7 @@ fn load_folder( } #[allow(clippy::too_many_arguments)] -fn add_hydrated( +pub(super) fn add_hydrated( source: &mut SourceReview, collected: &[Value], sent: bool, @@ -996,6 +1045,7 @@ fn add_hydrated( for (_, sent_time, result) in results { match result { Ok((mut message, cached)) => { + message.provider = identity.provider; if !cached { identity.account.clone_into(&mut message.account); message.own_addresses.clone_from(&identity.addresses); @@ -1046,6 +1096,7 @@ fn group_url(id: &str, thread: Option<&str>) -> Result { fn load_group(http: &Client, token: &str, address: &str) -> SourceReview { let mut source = SourceReview { + provider: MailProvider::Microsoft, label: format!("Group: {address}"), messages: vec![], errors: vec![], @@ -1107,10 +1158,12 @@ fn source_listing_failed(usable: usize, errors: &[ConnectionError]) -> bool { #[cfg(test)] mod tests { + use super::super::test_support::{concurrent_server, one_shot_server, scripted_server}; use super::*; fn synthetic_identity(addresses: &[&str]) -> UserIdentity { UserIdentity { + provider: MailProvider::Microsoft, account: "synthetic-account".into(), addresses: addresses.iter().map(|address| (*address).into()).collect(), display_name: Some("Synthetic User".into()), @@ -1260,7 +1313,7 @@ mod tests { fn next_page_rejected_has_a_distinct_message() { assert_eq!( ConnectionError::NextPageRejected.to_string(), - "Microsoft returned a next-page link outside the authorized collection; remaining pages were skipped." + "The mail service returned a next-page link outside the authorized collection; remaining pages were skipped." ); } #[test] @@ -1373,108 +1426,6 @@ mod tests { assert!(!is_event_message(&body)); } - /// Spins a real, one-shot loopback HTTP server (no TLS -- `fetch_event` - /// is exercised through [`fetch_from_origin`]'s testable seam, which - /// checks the request's origin against a caller-supplied one instead of - /// [`fetch`]'s hardcoded [`GRAPH_ORIGIN`]) that reads one request and - /// writes back `response` verbatim, then closes. Returns the server's - /// own origin (for use as both the request's base and the `expected_origin` - /// argument) and a handle the caller joins once the round trip is done. - fn one_shot_server(response: Vec) -> (String, std::thread::JoinHandle<()>) { - use std::io::{Read, Write}; - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let port = listener.local_addr().unwrap().port(); - let handle = std::thread::spawn(move || { - let (mut stream, _) = listener.accept().unwrap(); - let mut buffer = [0u8; 4096]; - let _ = stream.read(&mut buffer); - stream.write_all(&response).unwrap(); - let _ = stream.flush(); - }); - (format!("http://127.0.0.1:{port}/"), handle) - } - - fn scripted_server( - responses: Vec<(Duration, Vec)>, - ) -> ( - String, - std::sync::Arc, - std::thread::JoinHandle<()>, - ) { - use std::io::{Read, Write}; - use std::sync::{Arc, atomic::AtomicUsize}; - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let port = listener.local_addr().unwrap().port(); - let calls = Arc::new(AtomicUsize::new(0)); - let server_calls = Arc::clone(&calls); - let handle = std::thread::spawn(move || { - for (delay, response) in responses { - let (mut stream, _) = listener.accept().unwrap(); - server_calls.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - let mut buffer = [0u8; 4096]; - let _ = stream.read(&mut buffer); - std::thread::sleep(delay); - let _ = stream.write_all(&response); - let _ = stream.flush(); - } - }); - (format!("http://127.0.0.1:{port}/"), calls, handle) - } - - fn concurrent_server( - requests: usize, - delay: Duration, - ) -> ( - String, - std::sync::Arc, - std::sync::Arc, - std::thread::JoinHandle<()>, - ) { - use std::io::{Read, Write}; - use std::sync::{ - Arc, - atomic::{AtomicUsize, Ordering}, - }; - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let port = listener.local_addr().unwrap().port(); - let calls = Arc::new(AtomicUsize::new(0)); - let max_in_flight = Arc::new(AtomicUsize::new(0)); - let server_calls = Arc::clone(&calls); - let server_max = Arc::clone(&max_in_flight); - let handle = std::thread::spawn(move || { - let in_flight = Arc::new(AtomicUsize::new(0)); - let mut handlers = Vec::with_capacity(requests); - for _ in 0..requests { - let (mut stream, _) = listener.accept().unwrap(); - let in_flight = Arc::clone(&in_flight); - let calls = Arc::clone(&server_calls); - let max_in_flight = Arc::clone(&server_max); - handlers.push(std::thread::spawn(move || { - let mut buffer = [0u8; 4096]; - let _ = stream.read(&mut buffer); - calls.fetch_add(1, Ordering::SeqCst); - let now = in_flight.fetch_add(1, Ordering::SeqCst) + 1; - max_in_flight.fetch_max(now, Ordering::SeqCst); - std::thread::sleep(delay); - let _ = stream.write_all( - b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\n{}", - ); - let _ = stream.flush(); - in_flight.fetch_sub(1, Ordering::SeqCst); - })); - } - for handler in handlers { - handler.join().unwrap(); - } - }); - ( - format!("http://127.0.0.1:{port}/"), - calls, - max_in_flight, - handle, - ) - } - #[test] fn timeout_has_distinct_wording_after_one_retry() { let response = @@ -1499,7 +1450,7 @@ mod tests { assert_eq!(result, Err(ConnectionError::Timeout(1))); assert_eq!( result.unwrap_err().to_string(), - "Microsoft did not answer within 1 seconds." + "The service did not answer within 1 seconds." ); server.join().unwrap(); assert_eq!(calls.load(std::sync::atomic::Ordering::Relaxed), 2); @@ -1567,6 +1518,7 @@ mod tests { #[test] fn one_failed_body_fetch_does_not_drop_the_source() { let mut source = SourceReview { + provider: MailProvider::Microsoft, label: "Personal mailbox / Inbox".into(), messages: vec![], errors: vec![], @@ -1654,6 +1606,7 @@ mod tests { fetch_from_origin(&http, "synthetic-token", &list_url, &origin).unwrap(); let rows = synthetic_rows(1); let mut source = SourceReview { + provider: MailProvider::Microsoft, label: label.into(), messages: vec![], errors: vec![], @@ -1708,6 +1661,7 @@ mod tests { fn concurrent_hydration_preserves_listing_order() { let rows = synthetic_rows(8); let mut source = SourceReview { + provider: MailProvider::Microsoft, label: "Personal mailbox / Inbox".into(), messages: vec![], errors: vec![], @@ -1754,6 +1708,7 @@ mod tests { fn load_progress_reaches_the_discovered_row_count() { let rows = synthetic_rows(5); let mut source = SourceReview { + provider: MailProvider::Microsoft, label: "Personal mailbox / Inbox".into(), messages: vec![], errors: vec![], @@ -1792,6 +1747,7 @@ mod tests { fn concurrent_hydration_uses_at_most_four_requests() { let rows = synthetic_rows(8); let mut source = SourceReview { + provider: MailProvider::Microsoft, label: "Personal mailbox / Inbox".into(), messages: vec![], errors: vec![], @@ -1836,6 +1792,7 @@ mod tests { fn cached_hydration_makes_no_request_and_returns_the_item_unchanged() { let rows = synthetic_rows(1); let cached = MailItem { + provider: MailProvider::Microsoft, subject: "Cached subject".into(), body: "Cached synthetic body".into(), body_is_html: true, @@ -1870,6 +1827,7 @@ mod tests { .build() .unwrap(); let mut source = SourceReview { + provider: MailProvider::Microsoft, label: "Personal mailbox / Inbox".into(), messages: vec![], errors: vec![], @@ -1926,6 +1884,7 @@ mod tests { let rows = synthetic_rows(20); let mut source = SourceReview { + provider: MailProvider::Microsoft, label: "Personal mailbox / Inbox".into(), messages: vec![], errors: vec![], diff --git a/crates/openloops-graph/src/live/test_support.rs b/crates/openloops-graph/src/live/test_support.rs new file mode 100644 index 0000000..739a45e --- /dev/null +++ b/crates/openloops-graph/src/live/test_support.rs @@ -0,0 +1,177 @@ +use std::io::{Read, Write}; +use std::sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, +}; +use std::time::Duration; + +pub(super) fn one_shot_server(response: Vec) -> (String, std::thread::JoinHandle<()>) { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + let mut buffer = [0u8; 4096]; + let _ = stream.read(&mut buffer); + stream.write_all(&response).unwrap(); + let _ = stream.flush(); + }); + (format!("http://127.0.0.1:{port}/"), handle) +} + +pub(super) trait ScriptedResponse { + fn parts(self) -> (Duration, Vec); +} + +impl ScriptedResponse for Vec { + fn parts(self) -> (Duration, Vec) { + (Duration::ZERO, self) + } +} + +impl ScriptedResponse for (Duration, Vec) { + fn parts(self) -> (Duration, Vec) { + self + } +} + +pub(super) fn scripted_server( + responses: Vec, +) -> (String, Arc, std::thread::JoinHandle<()>) { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let calls = Arc::new(AtomicUsize::new(0)); + let server_calls = Arc::clone(&calls); + let handle = std::thread::spawn(move || { + for response in responses { + let (delay, response) = response.parts(); + let (mut stream, _) = listener.accept().unwrap(); + server_calls.fetch_add(1, Ordering::Relaxed); + let mut buffer = [0u8; 4096]; + let _ = stream.read(&mut buffer); + std::thread::sleep(delay); + let _ = stream.write_all(&response); + let _ = stream.flush(); + } + }); + (format!("http://127.0.0.1:{port}/"), calls, handle) +} + +pub(super) fn concurrent_server( + requests: usize, + delay: Duration, +) -> ( + String, + Arc, + Arc, + std::thread::JoinHandle<()>, +) { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let calls = Arc::new(AtomicUsize::new(0)); + let max_in_flight = Arc::new(AtomicUsize::new(0)); + let server_calls = Arc::clone(&calls); + let server_max = Arc::clone(&max_in_flight); + let handle = std::thread::spawn(move || { + let in_flight = Arc::new(AtomicUsize::new(0)); + let mut handlers = Vec::with_capacity(requests); + for _ in 0..requests { + let (mut stream, _) = listener.accept().unwrap(); + let in_flight = Arc::clone(&in_flight); + let calls = Arc::clone(&server_calls); + let max_in_flight = Arc::clone(&server_max); + handlers.push(std::thread::spawn(move || { + let mut buffer = [0u8; 4096]; + let _ = stream.read(&mut buffer); + calls.fetch_add(1, Ordering::SeqCst); + let now = in_flight.fetch_add(1, Ordering::SeqCst) + 1; + max_in_flight.fetch_max(now, Ordering::SeqCst); + std::thread::sleep(delay); + let _ = stream.write_all( + b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\n{}", + ); + let _ = stream.flush(); + in_flight.fetch_sub(1, Ordering::SeqCst); + })); + } + for handler in handlers { + handler.join().unwrap(); + } + }); + ( + format!("http://127.0.0.1:{port}/"), + calls, + max_in_flight, + handle, + ) +} + +pub(super) fn routed_server( + routes: Vec<(&'static str, Vec)>, +) -> (String, Arc, std::thread::JoinHandle<()>) { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let calls = Arc::new(AtomicUsize::new(0)); + let server_calls = Arc::clone(&calls); + let handle = std::thread::spawn(move || { + for _ in 0..routes.len() { + let (mut stream, _) = listener.accept().unwrap(); + let mut request = [0u8; 4096]; + let read = stream.read(&mut request).unwrap_or(0); + server_calls.fetch_add(1, Ordering::Relaxed); + let head = String::from_utf8_lossy(&request[..read]); + let path = head.split_whitespace().nth(1).unwrap_or_default(); + let response = routes + .iter() + .find(|(prefix, _)| path.starts_with(prefix)) + .map_or_else( + || { + b"HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + .to_vec() + }, + |(_, response)| response.clone(), + ); + let _ = stream.write_all(&response); + let _ = stream.flush(); + } + }); + (format!("http://127.0.0.1:{port}/"), calls, handle) +} + +#[test] +fn routed_server_selects_by_path_prefix_in_any_order() { + let response = |body: &str| { + format!( + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ) + .into_bytes() + }; + let (origin, calls, server) = routed_server(vec![ + ("/first", response("one")), + ("/second", response("two")), + ]); + let client = reqwest::blocking::Client::builder() + .no_proxy() + .build() + .unwrap(); + assert_eq!( + client + .get(format!("{origin}second/item")) + .send() + .unwrap() + .text() + .unwrap(), + "two" + ); + assert_eq!( + client + .get(format!("{origin}first/item")) + .send() + .unwrap() + .text() + .unwrap(), + "one" + ); + server.join().unwrap(); + assert_eq!(calls.load(Ordering::Relaxed), 2); +} diff --git a/docs/plans/2026-09-27-google-provider-and-shared-registration.md b/docs/plans/2026-09-27-google-provider-and-shared-registration.md new file mode 100644 index 0000000..08a7f23 --- /dev/null +++ b/docs/plans/2026-09-27-google-provider-and-shared-registration.md @@ -0,0 +1,404 @@ +# Plan: Google (Gmail + Tasks) provider and multi-tenant Microsoft distribution + +Date: 2026-09-27. Branch base: `main`. One feature branch and PR per phase. Codex executes; +Claude reviews and verifies (see "Execution notes"). + +## Context + +OpenLoops connects one Microsoft 365 work/school account through a user-entered Entra +client ID, the `organizations` authority, PKCE, and a memory-only token. The live code is +Graph-shaped end to end: `MailItem` carries Graph ids, links are gated to Outlook hosts, +reminders are Microsoft To Do, one global `SESSION`, and PowerShell governance checkers +confine network code to `openloops-graph`. Non-Microsoft mail is `outside_mvp` in +`contracts/support/support-matrix.json:253-263`. + +The owner wants: (1) people at other domains with Microsoft 365 can use the app without +registering their own Entra app; (2) Gmail users can use the app with Google Tasks +reminders; (3) one user can connect Microsoft and Google at the same time. + +Owner decisions taken during planning (2026-09-27): + +| Question | Decision | +|---|---| +| Microsoft distribution | Ship a shared multitenant client ID, complete publisher verification, add an admin-consent link, keep the BYO client-ID field as fallback. ADR-012 amendment. | +| Google audience | Personal Gmail first. One external Google OAuth client shipped in the app, in **Testing** status (free; 100 named test users; unverified-app screen). Verification + CASA deferred until the owner chooses to pay. | +| Google reminders | Google Tasks, own phase. | +| Accounts | Microsoft and Google connected at once, merged review. Last phase. | +| Order | PR-1 abstraction + shared registration → PR-2 Gmail read + review → PR-3 Google Tasks → PR-4 dual accounts. Console work (Phase M1, G0) runs in parallel. | + +## Answer to "do I need to publish?" + +No store listing and no Microsoft 365 App Certification. Three things are needed: + +1. **Multitenant registration.** Already true ("accounts in any organizational directory"). Free. +2. **Publisher verification.** Free, no license. Prerequisites: Partner Center account in the + Microsoft AI Cloud Partner Program (free tier; business verification 3 to 5 business + days); the registration owned by a work/school tenant associated with that Partner + global account; publisher domain = a DNS-verified custom domain in that tenant (not + `*.onmicrosoft.com`); the verifying user holds Application Administrator in Entra and + Partner Admin or Account Admin in Partner Center, signed in with MFA. Without it, apps + registered after 2020-11-08 show "unverified" in other tenants and, under default + risk-based step-up consent, users there cannot consent at all. +3. **Admin consent path.** `Mail.Read` is not low-impact by default, so many tenants need an + admin to consent even to a verified publisher. `Group.ReadBasic.All` and + `Group-Conversation.Read.All` always need admin consent. The app must show the + admin-consent URL. + +Cost (verified on learn.microsoft.com and support.google.com, 2026-09-27): + +| Item | Cost | +|---|---| +| Entra registration, multitenant setting, publisher verification | $0 | +| Partner Center membership, base tier | $0 | +| Custom domain | owner already has one | +| Google Cloud project, OAuth client, Testing status, Google's review | $0 | +| CASA lab assessment, only when leaving Testing with the restricted Gmail scope | ~$500 to $4,500 per year, deferred | + +## Phase M1: Microsoft console work (owner, no code) + +1. Pick the publisher tenant; add and DNS-verify the custom domain. +2. Enroll in the Microsoft AI Cloud Partner Program in Partner Center; complete business + verification; record the Partner One ID (global account, not a location ID). +3. Associate the Entra tenant that owns the OpenLoops registration with that account. +4. Registration: publisher domain = custom domain; account type = any organizational + directory; redirect `http://localhost` under Mobile and desktop; remove unused + platforms; branding (name, logo, homepage, privacy statement, terms, support contact). + Delegated permissions: `User.Read`, `Mail.Read`, `Mail.Read.Shared`, `Tasks.ReadWrite`, + `Group.ReadBasic.All`, `Group-Conversation.Read.All`. +5. Entra portal → Branding & properties → Publisher verification → enter Partner One ID. +6. Add a second registration owner; write the owner-recovery note (ADR-012 control). + +## Phase G0: Google console work (owner, no code) + +1. Google Cloud project "OpenLoops"; enable Gmail API and Google Tasks API. +2. OAuth consent screen: External; Testing; app name, support email, homepage, privacy + policy URL; scopes `openid`, `email`, `profile`, + `https://www.googleapis.com/auth/gmail.readonly`, `https://www.googleapis.com/auth/tasks`. +3. Add test users by Gmail address (cap 100). Owner first. +4. OAuth client, type Desktop app. Record client ID and secret for build-time injection + only. Google documents the desktop client secret as not confidential; it still never + enters the repository. +5. Behaviours to document: unverified-app interstitial once per user; 7-day authorization + expiry (moot: the app keeps no refresh token); 100-user cap. +6. Upgrade path, deferred and paid: verification submission, CASA at the assurance level + Google assigns, annual renewal. + +## Design constraints that shape the code + +- **Google code lives inside `openloops-graph` under `src/live/google/`.** No new crate, + no rename. Reasons: `check-authentication-boundary.ps1:227-256`, + `check-incremental-authorization.ps1:191-213`, `check-synchronization-boundary.ps1:67` + allow network deps and `TcpStream`/HTTP symbols only in `openloops-graph` + `src/{callback.rs,live.rs,live/*}`; `check-protected-state-boundary.ps1:380` + fingerprints every `Cargo.toml`, `Cargo.lock`, and `openloops-desktop/src/main.rs` but + not `live/*`. Add a module-doc note in `live.rs` naming the Google submodule. +- **No new Cargo dependencies in any PR.** base64url via `crate::encoding`, JSON via + `serde_json`, dates via `chrono`. Check `git diff --stat Cargo.lock` is empty per PR. +- **Microsoft HMAC inputs do not change.** `Decisions::fingerprint` (`loop_state.rs:97-122`) + and `Relations::thread_key` (`link_state.rs:107`) take `account` and ids as strings; + Microsoft `account` stays the bare Graph `/me.id`. Google accounts are + `"google:" + OIDC sub`. +- **Reuse controls as modes; no new button kinds** (repo rule). +- **Shipped identifiers are build-time injected**, never committed: + `option_env!("OPENLOOPS_MS_CLIENT_ID")`, `option_env!("OPENLOOPS_GOOGLE_CLIENT_ID")`, + `option_env!("OPENLOOPS_GOOGLE_CLIENT_SECRET")`. Source builds without them behave as + today (BYO only). Keeps ADR-012's "no real client ID in the repository" literally true + and the public-repo canary green (its secret rule at `check-public-repo.ps1:153` + matches only quoted literals). + +## PR-1: provider abstraction + shared registration (`feat/mail-provider-abstraction`) + +New `crates/openloops-graph/src/live/provider.rs`: +- `enum MailProvider { Microsoft (default), Google }` with `service_name()` + ("Microsoft 365" / "Google"), `mail_client_name()` ("Outlook" / "Gmail"), + `tasks_name()`, `tasks_url()`, `is_trusted_message_link(url)`. +- `enum AccountConfig { Microsoft(ConnectionConfig), Google(GoogleConfig) }` with + `provider()`, `check_connection()`, `load_recent_with()`, `load_sources_with()`. Enum + dispatch, not trait objects. `GoogleConfig` is a stub in PR-1 (filled in PR-2). +- `struct ProviderLoad { provider, sources: Result, ConnectionError> }` + and `load_all(&[AccountConfig], cache, progress) -> Vec`. + +New `crates/openloops-graph/src/live/registration.rs`: +- `microsoft() -> Option` from `option_env!`, + validated by the existing `valid_application_id` (`live.rs:211-221`). +- `google() -> Option`. +- `microsoft_admin_consent_url(client_id) -> String`: + `https://login.microsoftonline.com/organizations/v2.0/adminconsent?client_id={id}&scope={scopes}&redirect_uri=http%3A%2F%2Flocalhost`. + Document that after consent the admin's browser lands on an unreachable localhost page; + consent is still recorded. +- Unit test: values come only from `option_env!`; precedence BYO field > injected > none. + +Neutral fields (`live/review.rs`): add `provider: MailProvider` to `MailItem` (17-38), +`SourceReview` (55-64), `UserIdentity` (41-46). Default is Microsoft so `synthetic()` +(`review_scan.rs:7719`) and fixtures compile unchanged. `ReviewMessage` +(`review_scan.rs:51-74`) gains `provider`, copied in `prepare()` (2145). + +Sessions (`live.rs`): `static SESSIONS: Mutex<[Option; 2]>` indexed by provider; +`clear_session(provider)`, `clear_all_sessions()`, `has_session(provider)`. `CONNECTING` +stays one global. Callers: `app_model.rs:349` → `clear_all_sessions()`; `slint_ui.rs:827` +→ `clear_session(Microsoft)`; `slint_review.rs:1629,1971` → per provider. + +OAuth parametrisation (`live.rs`): `struct OAuthEndpoints { authorize, token, +redirect_host, client_secret: Option, extra_params, normalize_scope }`; +`authorize_with(endpoints, client_id, scopes)`. The token-URI confinement at `live.rs:425` +compares against `endpoints.token`. `callback.rs`: `enum RedirectHost { Localhost, +LoopbackIp }`; `Listener::bind(host)`; `redirect_uri()` and the `Host` header check +follow it. Microsoft = `Localhost` (matches its registration); Google = `LoopbackIp`. + +`ConnectionError` Display (`live.rs:84-122`): provider-neutral wording ("The mail service +returned HTTP 403"). Update the three asserted strings in the test at `live.rs:740`. + +Deep links: `is_outlook_link` (`app_model.rs:1161`) → `is_trusted_message_link` accepting +the four Outlook prefixes plus `https://mail.google.com/`; `gated_outlook_url` +(`slint_review.rs:230`) → `gated_message_url`; `on_open_external` (2487) accepts any +`MailProvider::tasks_url()`. Slint structs carrying `url` gain `url-label`; Rust fills +"Open message in {mail_client_name}" (replaces literals at `review.slint:393,402,408,562,626`). + +`AccountDisplay::connected(&[MailProvider])` (`app_model.rs:1183`): "Signed in · +Microsoft 365", "· Google", or "· Microsoft 365 + Google"; `chrome.slint:38` binds a new +`account-text` property. + +Scan-scope collision fix now: `ConversationKey = (account, conversation)` replaces the bare +conversation-id sets in `ScanScope::Incremental` (`review_scan.rs:1687-1707, 3262, 4323`), +`Outcome::{RetryMail,RetryScan,CheckScan}`, `append_sources` return, `merge_scan`, +`retryable_conversations`, `prior_open_items`. + +Microsoft-only heuristics: `merge_threads_with_rules` (`review_scan.rs:7023`) gets +`authoritative: bool` on `ThreadGroup`, true for non-Microsoft; never merges those. +`event` stays Microsoft-only. + +`Reminder::Created/Completed` (`loop_state.rs:51-76`) gain `provider`; encoding bytes 0..3 +unchanged (Microsoft), 4/5 = Google. `MAGIC` unchanged. + +Desktop renames: `Outcome::Microsoft(..)` → `Outcome::Connection(MailProvider, ..)`; +`Service::Microsoft` → `Service::Mailbox`; `AppModel` gains `google: Status`. +`AppModel::effective_microsoft_client_id()` = BYO trimmed if non-empty, else +`registration::microsoft()`. + +Sources UI (`ui/sources.slint:141-160`): with a shipped ID, the client-ID field moves into +a disclosure "Use my organization's own registration" (pattern at 147-152), and a read-only +monospace `Field { label: "Admin consent link" }` shows the URL (Slint has no clipboard +without a dependency; users select-and-copy). "Open Microsoft Entra" stays for BYO. + +Consent-failure mapping: AADSTS65001, 90094, 650052/650056 → fixed content-free +`ConnectionError` texts naming admin consent or BYO. Tests via `scripted_server`. + +Settings (`settings.rs`, append-only field ladder, no v2 record): field 13 +`google_client_id`, 14 `google_client_secret` (`Zeroizing`), 15 `mail_providers` +tag (`ms`, `google`, `ms,google`; default `ms`). Roughly +130 bytes against the 2,560 cap. +Extend the boundary test. + +Governance (owner-approved commit, same PR): ADR-012 amendment (shared registration +enabled via build-time injection; BYO fallback; admin-consent link); +`contracts/distribution/registration-boundary.json` status strings and +`shared_registration_enabled`; `tools/check-distribution-registration-boundary.ps1` phrase +list (line 154), status literals (71-73), governance capability +`shared_project_registration.state`, and hash literals (41, 148-153, which also pin +`product-spec.md`, `implementation-plan.md`, `prd-traceability.md`); +`docs/native-setup.md:91-93`, `docs/live-connection.md:7-21`. + +## PR-2: Google OAuth + Gmail loader + review (`feat/google-gmail-provider`) + +Carried over from the PR-1b review: make `Service::Mailbox` carry the provider +(`Service::Mailbox(MailProvider)`) so a worker panic during a Google check is reported on +the Google status, not the Microsoft one (`app_model.rs` `pending_disconnected`); rename +`microsoft_status()` to a provider-neutral name now that it formats Google reports too. + +Files: `live/google/mod.rs` (`GoogleConfig`, endpoints, scopes, `with_google_session`, +`check_connection`), `live/google/gmail.rs` (identity, listing, hydration, `load_*`), +`live/google/mime.rs`, `live/test_support.rs` (`#[cfg(test)]`, fake servers moved from +`review.rs:1383-1476`, deduplicating `reminders.rs:336`, plus a path-routed +`routed_server` for out-of-order hydration workers). + +OAuth: authorize `https://accounts.google.com/o/oauth2/v2/auth`, token +`https://oauth2.googleapis.com/token`; `BasicClient` with `set_client_secret` and +`AuthType::RequestBody`; extras `access_type=online`, `prompt=select_account`, +`include_granted_scopes=true`. Google returns a refresh token to installed apps regardless +of `access_type`: drop and zeroize it immediately; store only `access_token`/`expires_at`. +`GoogleConfig::new(client_id, secret)` validates `^\d+-[a-z0-9]+\.apps\.googleusercontent\.com$` +and a non-empty control-free secret. Scopes: `openid email profile` + `gmail.readonly` +(mail) or `tasks` (reminders); the existing scope-union logic (`live.rs:321-346`) gives +incremental consent. + +Gmail flow (`gmail::load_sources_with`), all functions taking `origin` for tests: +1. `GET https://openidconnect.googleapis.com/v1/userinfo` → `UserIdentity { account: + "google:{sub}", addresses: [email], display_name, given_name }`. Only the primary + address is known (send-as aliases need `gmail.settings.basic`, not requested; document). +2. Per label `INBOX`, `SENT`: `GET /gmail/v1/users//messages?labelIds={L}&q=newer_than:30d&maxResults=100[&pageToken]` + (`` is the literal segment `me`; written this way because the repo gate rejects the bare path), + ≤10 pages, `partial` when the 100 cap is hit with a `nextPageToken`. +3. Reuse `add_hydrated` (`review.rs:942`, made `pub(super)`; needs only `id`). Cache hits + on `(account, id)` skip the fetch. Misses: `GET /gmail/v1/users//messages/{id}?format=full`. +4. `hydrate` → `MailItem { provider: Google, id, conversation: threadId, web_link: + https://mail.google.com/mail/u/0/#all/{id}, sent: label SENT, team: false, event: None, + received: internalDate → RFC3339 UTC }`; drop rows older than `cutoff_timestamp()`; + `ResponseTooLarge` → `MessageTooLarge` (parity with the 1 MiB bound). +5. `fetch_from_origin_with_policy` (`review.rs:308`) → new + `fetch_from_origin_with_headers(.., headers)`; Microsoft passes the `Prefer` pair, + Gmail passes none. + +`mime.rs`: `select_body(payload)` depth-first over `parts`, skip attachments (non-empty +`filename` or `Content-Disposition: attachment`), prefer `text/html` then `text/plain`; +`decode_part` base64url via `crate::encoding`, charsets utf-8, iso-8859-1, windows-1252, +else lossy; 131,072-char bound like `item()` (`review.rs:564`); `header()` case-insensitive; +`decode_rfc2047` B/Q for utf-8/latin1; `parse_address_list` matching `item()` formatting. + +Source labels: `"Gmail / Inbox"`, `"Gmail / Sent"` (cannot collide with Microsoft labels; +`source_selected`'s `rsplit_once(" / ")` keeps working). + +Sources UI: card 1 becomes "Connect your mailbox" with a `Segmented { "Microsoft 365", +"Google" }` (pattern at `sources.slint:167`). Google mode: optional `google-client-id` and +`google-client-secret` overrides (password `Field` with the existing "Show key" pattern), +`LinkButton "Open Google Cloud console"`, helper text about the unverified-app screen and +test-user list. One `PrimaryButton "Sign in & check inbox"` acts on the selected mode. +Per-mode status lines. Plumb through `ui/app.slint` and `slint_ui.rs:262-285,365`. + +Governance (owner-approved): ADR-002 amendment defining "client secret" as +confidential-client material that authenticates the binary; the Google installed-app +parameter is a non-confidential registration parameter and PKCE remains required; +`contracts/identity/authentication-boundary.json` adds a `providers` block (Google +origins; `client_secret_handling: non_confidential_installed_app_parameter`) while +`confidential_client_material` stays prohibited; `check-authentication-boundary.ps1` +hash re-pin. ADR-001:43 clarifying sentence. ADR-003: Google scope table (prefer a new +`contracts/identity/google-scope-boundary.json` over editing the exact-set +`permission_rows`). New `docs/adr/ADR-015-google-provider.md` (abstraction, Testing-status +registration, personal-Gmail target, no refresh token, `google:` prefix, MIME limits) + +`contracts/governance/capabilities.json` + `check-governance.ps1` ADR inventory. +`contracts/support/support-matrix.json`: split `non_microsoft_mailbox` into +`google_personal_gmail` (`gated_target`, `disabled`) and the rest; `check-support-matrix.ps1:123` +exact rule + hash. `docs/product-spec.md:657` drop "Gmail" and "multiple accounts" +(re-pins `check-distribution…:151`). Docs: `live-connection.md`, `native-setup.md` +(env-var build instructions), `support-matrix.md:41`, `inbox-review.md`. + +## PR-3: Google Tasks reminders (`feat/google-tasks-reminders`) + +`live/google/tasks.rs` mirroring `reminders.rs`: `create`, `complete`, `check_status`, +reusing `ReminderRequest` (gains `provider`), `ReminderOutcome`, `ReminderFailure`, +`ReminderCompletionOutcome`, `TaskStatusOutcome`; `valid_graph_id` → `valid_remote_id`. +- `create`: session with `tasks` scope → userinfo `sub` must equal `request.account` + (`AccountMismatch`) → `GET /tasks/v1/users/<@me>/lists?maxResults=100` (angle brackets are not literal; the repo gate rejects the bare path), first list is the + default (`DefaultListNotFound` if empty or paging incomplete) → `POST /tasks/v1/lists/{list}/tasks` + `{title, status: "needsAction", due: "YYYY-MM-DDT00:00:00.000Z", notes: "Created after + review in OpenLoops.\nReminder time: {local}\nOpenLoops reference: {marker}"}`. Success is + HTTP 200. Store the concrete list id. +- `complete`: `PATCH .../tasks/{t}` `{"status":"completed"}` → 200. `check_status`: `GET`; + 404 → not found. + +Due-time limitation (Google records date only, no alert) surfaced in: the review draft +(`review.slint:469-486`, new `reminder-service` and `reminder-time-note` properties; Google +text "Google Tasks saves the date only and does not alert you; the time you pick is written +into the task's notes"), `reminder_outcome` (`app_model.rs:736-738`) via `tasks_name()`, +and reconcile links (`review.slint:495,508`) via per-card `tasks_url()`. + +Desktop dispatch: `ReminderDraft` (`review_model.rs:46`) gains `provider`; +`on_create_reminder` (`slint_review.rs:2437`), `dispatch_pending_reminder` (1706), the +Handled-completion path (1791-1807), `dispatch_reminder_sync` (`app_model.rs:955-986`) +build an `AccountConfig` via new `AppModel::account_config(provider)`; +`reminder_sync_checks` (`review_model.rs:1059`) groups by provider. + +Governance (owner-approved): ADR-009 amendment + `contracts/reminder/adapter-boundary.json` +Google Tasks row with the date-only limitation; `check-reminder-adapter-boundary.ps1` +hash re-pins (two SHA-256 sites). + +## PR-4: both providers at once (`feat/dual-provider-review`) + +- `start_mail_load` (`slint_review.rs:1608`) builds `Vec` from + `AppModel::enabled_accounts()`; `Outcome::Mail(Vec)`, `CheckMail { loads }`, + `RetryMail { loads, .. }`. `check_mail_outcome` (`app_model.rs:609`) extends the cache + and appends sources from every `Ok`, records one status line per `Err` prefixed with + `service_name()`; one provider's failure no longer discards the other's results. +- `LoadProgress::begin(total)` once in `load_all`; loaders only increment. +- `ReviewState.failed_sources: BTreeSet<(MailProvider, String)>`; `on_retry_failed` + (`slint_review.rs:1938`) groups labels by provider. +- Sources UI: one `Checkbox "Include in scans"` per mode bound to `mail_providers`; + heading pills "Microsoft connected" / "Google connected". +- Sign-ins run sequentially inside the one job (single `CONNECTING`). Busy copy: "Complete + Microsoft and Google sign-in; then downloading…". +- Governance: `contracts/governance/capabilities.json` row if a dual-account capability is + added; ADR-001:46 amendment ("multiple accounts" no longer excluded for the two-provider + case). + +## Verification + +Per PR, in this order, reading each result before committing (never gate and commit in +one command): + +``` +cargo fmt --all -- --check +cargo clippy -p openloops-graph --features live-connection --all-targets -- -D warnings +cargo clippy -p openloops-desktop --features native-ui,ui-screenshot --all-targets -- -D warnings +cargo test -p openloops-graph --features live-connection --offline +cargo test -p openloops-desktop --features native-ui --offline -- --test-threads=1 +pwsh ./tools/check-authentication-boundary.ps1 +pwsh ./tools/check-incremental-authorization.ps1 +pwsh ./tools/check-synchronization-boundary.ps1 +pwsh ./tools/check-evidence-identity-boundary.ps1 +pwsh ./tools/check-protected-state-boundary.ps1 +pwsh ./tools/check-support-matrix.ps1 +pwsh ./tools/check-distribution-registration-boundary.ps1 +pwsh ./tools/check-reminder-adapter-boundary.ps1 +pwsh ./tools/check-governance.ps1 +git add -A; pwsh ./tools/check-public-repo.ps1 -Mode Staged +git diff --stat main -- Cargo.lock # must be empty +``` + +Tests added per PR: +- PR-1: per-provider session slots; `authorize_with` rejects the other provider's token + URI; `RedirectHost::LoopbackIp` host check; two accounts sharing a conversation id scan + only the targeted one; `merge_threads` never merges Google groups; reminder bytes 2/3 + decode Microsoft and 4/5 Google with old fixture bytes unchanged; + `is_trusted_message_link` accepts `mail.google.com` and rejects `https://evil/mail.google.com/`; + `AccountDisplay` three names; settings fields 13–15 boundaries; registration precedence; + AADSTS error mapping. +- PR-2: `routed_server` cases: userinfo → two folders → three messages (html + multipart/alternative, plain-only, over-size → `MessageTooLarge`); paging to the 100 cap; + cutoff drop; cache hit skips the GET (assert call count); 401 → `Unauthorized`; bad + base64 → `ResourceUnavailable`; RFC 2047 subject; latin1 body; attachments skipped; + `web_link` format; `conversation == threadId`; `sent` for SENT; `GoogleConfig::new` + rejects malformed ids; `prepare()` labels "Open message in Gmail". +- PR-3: account mismatch makes no POST; empty list set → `DefaultListNotFound`; 200 → + `Created { provider: Google }`; PATCH/GET paths; body has date-only `due` and the time in + `notes`; per-provider `reminder_outcome` text; `reminder_sync_checks` grouping. +- PR-4: `load_all` continues after one provider fails; mixed `Ok`/`Err` in + `check_mail_outcome`; retry routing by `(provider, label)`; progress totals; title text. + +Manual smoke, owner-run, free (real accounts, no paid API): build with +`OPENLOOPS_MS_CLIENT_ID`, `OPENLOOPS_GOOGLE_CLIENT_ID`, `OPENLOOPS_GOOGLE_CLIENT_SECRET` +set; confirm the Google interstitial for a listed test user, a Gmail thread grouped by +`threadId`, a created Google Task showing the date with the time in notes; then the +Microsoft fresh-tenant check (a free second Entra tenant with default consent settings: +verified badge visible; user consent succeeds or the "needs admin approval" page appears +and the admin-consent URL works). Record outcomes in `docs/live-connection.md`. + +## Risks + +1. **`client_secret` wording.** ADR-001/002 prohibit "a client secret". Resolution is the + ADR-002 definition amendment above; the parameter authenticates nothing beyond PKCE. +2. **Gmail 1 MiB responses.** `format=full` inlines small attachment bodies; oversize + messages are skipped as `MessageTooLarge`. Document in `inbox-review.md`. +3. **Charset fidelity.** Hand-rolled decoding covers UTF-8, Latin-1, Windows-1252, B/Q + encoded words; others fall back to lossy UTF-8. Documented limitation; a dependency would + re-pin the protected-state fingerprint. +4. **`main.rs` is fingerprinted.** Keep `openloops_graph::live::{ConnectionConfig, + SharedScope, check_connection}` exported under those names. +5. **Testing status ceiling.** 100 named users; the owner adds each address by hand. + Leaving Testing is the paid CASA path. +6. **Checker edits.** A classifier blocks Claude from editing `check-*.ps1`; Codex makes + those edits, and each lands in an owner-approved commit with the re-run `test-*.ps1`. +7. **Gmail path literals trip the public-repo gate.** `tools/check-public-repo.ps1`'s + "personal Unix home-directory path" rule (line 173) matches any `/users//` segment, + case-insensitively, so the Rust literals for the Gmail and Tasks `users//...` and + `users/<@me>/...` paths in PR-2/PR-3 will be blocked. Resolution, owner-approved in + PR-2: add `me/` and `@me/` to that rule's negative lookahead (alongside `Shared/`, + `runner/`, `sandbox/`), with a test in `tools/test-public-repo.ps1` (or equivalent) + showing a real home path is still caught. Do not build the path from pieces to dodge + the gate. + +## Execution notes + +- First execution step: commit this design as + `docs/plans/2026-09-27-google-provider-and-shared-registration.md` on the PR-1 branch. +- Codex does the implementation (`codex exec -c model_reasoning_effort=medium`, worktree + per PR). Claude reviews diffs, runs the gates, and inspects test output before any + "done" claim (`superpowers:verification-before-completion`). +- Do not skip or reorder PR phases without asking the owner.