Skip to content
Merged
11 changes: 10 additions & 1 deletion src/core/jsonrpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1668,9 +1668,18 @@ async fn run_server_inner(
});

// Check if a user is already logged in from a previous session.
// Resolving the id also warms the Sentry scope on boot-restore
// (via `warm_sentry_user_from_active_session`) so background-loop
// errors — e.g. the periodic Composio sync tick — carry the user
// id. `store_session` only fires on a fresh login, so without
// this a restored session would report `user = None`.
let already_logged_in = crate::openhuman::config::default_root_openhuman_dir()
.ok()
.and_then(|root| crate::openhuman::config::read_active_user_id(&root))
.and_then(|root| {
crate::openhuman::credentials::ops::warm_sentry_user_from_active_session(
&root,
)
})
.is_some();

if already_logged_in {
Expand Down
57 changes: 57 additions & 0 deletions src/core/observability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1749,6 +1749,63 @@ pub(crate) fn report_error_message(
);
}

/// Bind the signed-in user's id to the process-global Sentry scope so every
/// subsequently-reported event carries `user.id`.
///
/// Why this exists: the `before_send` hooks in `src/main.rs` and the Tauri
/// shell already pluck `user.id` from `app_state::peek_cached_current_user_identity()`,
/// but that cache is only warmed by the frontend-driven `app_state_snapshot`
/// RPC. Errors raised from background loops — e.g. the periodic Composio sync
/// tick that emits the `[composio-direct]` shapes — fire with an empty cache,
/// so the event lands with no user attribution (userCount=0). Setting the
/// scope user eagerly at the session boundary (login / account switch) makes
/// the id available regardless of whether the snapshot cache was ever warmed,
/// mirroring how the backend tags every event at the session boundary.
///
/// Only the `id` is carried — never email, name, or IP — to stay consistent
/// with `send_default_pii: false` in the Sentry client options. The `id` is
/// the backend's mongo ObjectId for the user; it is recorded only on the
/// Sentry scope and never logged here.
///
/// No-op when the Sentry client is a no-op guard (missing DSN) — the scope
/// mutation is simply never flushed.
pub fn set_sentry_user_id(user_id: &str) {
let trimmed = user_id.trim();
if trimmed.is_empty() {
tracing::debug!(
target: REPORT_ERROR_TRACING_TARGET,
"[observability] set_sentry_user_id skipped: blank id"
);
return;
}
let owned = trimmed.to_string();
sentry::configure_scope(|scope| {
scope.set_user(Some(sentry::User {
id: Some(owned.clone()),
..Default::default()
}));
});
// PII-safe: log only the id length, never the id itself.
tracing::debug!(
target: REPORT_ERROR_TRACING_TARGET,
id_len = owned.len(),
"[observability] Sentry scope user bound"
);
}

/// Clear the signed-in user from the process-global Sentry scope.
///
/// Called on logout so events reported after sign-out (or for a different
/// account before the next `set_sentry_user_id`) are not misattributed to the
/// previous user. Mirrors `set_sentry_user_id` — no-op under a no-op guard.
pub fn clear_sentry_user() {
sentry::configure_scope(|scope| scope.set_user(None));
tracing::debug!(
target: REPORT_ERROR_TRACING_TARGET,
"[observability] Sentry scope user cleared"
);
}

/// Returns true when a Sentry event is a per-attempt provider HTTP failure
/// that the reliable-provider layer already handles via retry + fallback.
///
Expand Down
85 changes: 85 additions & 0 deletions src/core/observability_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3547,3 +3547,88 @@ fn report_error_or_expected_routes_channel_supervisor_restart_through_expected_p
&[("channel", "discord")],
);
}

#[test]
fn set_sentry_user_id_attaches_id_to_reported_events() {
use std::sync::Arc;

use sentry::test::TestTransport;

// Stand up an isolated hub with a TestTransport so we can observe the
// user the captured event carries. Bound to the current thread only via
// `HubSwitchGuard`, so it never bleeds into other tests.
let transport = TestTransport::new();
let options = sentry::ClientOptions {
dsn: Some("https://public@sentry.invalid/1".parse().unwrap()),
transport: Some(Arc::new(transport.clone())),
..Default::default()
};
let hub = Arc::new(sentry::Hub::new(
Some(Arc::new(options.into())),
Arc::new(Default::default()),
));
let _guard = sentry::HubSwitchGuard::new(hub);

// Mimics the mongo ObjectId the backend hands us at the session boundary.
set_sentry_user_id("507f1f77bcf86cd799439011");
report_error(
"simulated direct-mode failure",
"composio",
"list_connections",
&[],
);

let events = transport.fetch_and_clear_events();
assert_eq!(events.len(), 1, "expected exactly one captured event");
let user = events[0]
.user
.as_ref()
.expect("captured event should carry scope user");
assert_eq!(user.id.as_deref(), Some("507f1f77bcf86cd799439011"));
// PII-minimal: only the id is ever set on the scope user.
assert!(user.email.is_none());
assert!(user.username.is_none());

// Logout clears the scope; subsequent events must not be misattributed.
clear_sentry_user();
report_error("post-logout failure", "composio", "list_connections", &[]);
let events = transport.fetch_and_clear_events();
assert_eq!(events.len(), 1);
assert!(
events[0].user.is_none(),
"scope user must be cleared after clear_sentry_user()"
);
}

#[test]
fn set_sentry_user_id_ignores_blank_input() {
use sentry::test::TestTransport;
use std::sync::Arc;

// Blank / whitespace-only ids are dropped rather than attaching an empty
// `user.id` that would still count as a distinct user in Sentry. Assert the
// emitted event carries no user — without this the test would pass even if
// a blank id were bound to the scope.
let transport = TestTransport::new();
let options = sentry::ClientOptions {
dsn: Some("https://public@sentry.invalid/1".parse().unwrap()),
transport: Some(Arc::new(transport.clone())),
..Default::default()
};
let hub = Arc::new(sentry::Hub::new(
Some(Arc::new(options.into())),
Arc::new(Default::default()),
));
let _guard = sentry::HubSwitchGuard::new(hub);

set_sentry_user_id("");
set_sentry_user_id(" ");
report_error("blank-id failure", "composio", "list_connections", &[]);

let events = transport.fetch_and_clear_events();
assert_eq!(events.len(), 1, "expected exactly one captured event");
assert!(
events[0].user.is_none(),
"blank id must not attach a scope user"
);
}
34 changes: 34 additions & 0 deletions src/openhuman/credentials/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,17 @@ pub async fn store_session(

logs.push("session stored".to_string());

// Bind the signed-in user's id to the Sentry scope only AFTER the session
// is persisted above — a failed `store_provider_token` must not leave the
// global scope pointing at a user whose login never completed. Covers
// fresh login and account switch (both route through here) so background
// -loop errors carry `user.id` even before the frontend `app_state_snapshot`
// RPC warms the identity cache. Only the id is sent — never email/name/IP.
// See `set_sentry_user_id`.
if let Some(ref uid) = resolved_user_id {
crate::core::observability::set_sentry_user_id(uid);
}

match crate::openhuman::memory::global::init(effective_config.workspace_dir.clone()) {
Ok(_) => logs.push(format!(
"memory client bound to workspace {}",
Expand Down Expand Up @@ -364,6 +375,10 @@ pub async fn clear_session(config: &Config) -> Result<RpcOutcome<serde_json::Val
}
}

// Drop the Sentry scope user so post-logout events aren't misattributed to
// the account that just signed out. The next `store_session` re-binds it.
crate::core::observability::clear_sentry_user();

// Stop all login-gated services (voice, autocomplete, screen
// intelligence, local AI) so they don't run as orphan processes after
// logout, consuming RAM/CPU with no user context to operate against.
Expand All @@ -381,6 +396,25 @@ pub async fn clear_session(config: &Config) -> Result<RpcOutcome<serde_json::Val
))
}

/// Warm the Sentry scope from the on-disk active session at boot.
///
/// `store_session` binds the Sentry user only on a fresh login / account
/// switch. A restored session (the app reopened while already signed in) never
/// routes through `store_session`, so without this its background-loop errors
/// (e.g. the periodic Composio sync tick) report with `user = None`. Called
/// from the boot path once an existing session is detected. Returns the active
/// user id when one is found. Only the id reaches Sentry — never email/name/IP.
///
/// Note: this does double duty — it returns the active user id *and* binds the
/// Sentry scope as a side effect, since the boot path needs both together. If a
/// future caller only wants the id without touching observability, read
/// `read_active_user_id` directly instead.
pub fn warm_sentry_user_from_active_session(root_dir: &std::path::Path) -> Option<String> {
let uid = read_active_user_id(root_dir)?;
crate::core::observability::set_sentry_user_id(&uid);
Some(uid)
}

pub async fn auth_get_state(
config: &Config,
) -> Result<RpcOutcome<super::responses::AuthStateResponse>, String> {
Expand Down
23 changes: 23 additions & 0 deletions src/openhuman/credentials/ops_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,3 +737,26 @@ async fn each_account_workspace_holds_its_own_credential_data() {
assert_eq!(result_a.value[0].provider, "anthropic");
assert_eq!(result_b.value[0].provider, "anthropic");
}

// ── warm_sentry_user_from_active_session (boot-restore) ────────

/// Boot-restore path: when `active_user.toml` holds an id, the warmer returns
/// it (and binds the Sentry scope) so background-loop errors carry `user.id`
/// without a fresh `store_session`.
#[test]
fn warm_sentry_user_from_active_session_returns_active_id() {
let tmp = TempDir::new().unwrap();
let root = tmp.path();
write_active_user_id(root, "507f1f77bcf86cd799439011").unwrap();

let id = warm_sentry_user_from_active_session(root);
assert_eq!(id.as_deref(), Some("507f1f77bcf86cd799439011"));
}

/// No on-disk session (signed out / fresh install): the warmer is a no-op and
/// returns `None` so the boot path defers service startup until login.
#[test]
fn warm_sentry_user_from_active_session_returns_none_without_active_user() {
let tmp = TempDir::new().unwrap();
assert_eq!(warm_sentry_user_from_active_session(tmp.path()), None);
}
Loading