Crash reporting: post-review fixes

vdavid · vdavid · commit be29affc98f2 · 2026-03-23T14:09:14.000+01:00
- Fix `serde(rename_all = "camelCase")` on `CrashReport` and `ActiveSettings` — frontend field access was silently broken
- Settings loaded via `settings::load_settings()` instead of hardcoded defaults
- Add `ai_provider` and `verbose_logging` to Rust `Settings` struct for crash reports
- macOS thread count via Mach `task_threads()` API (was always 0)
- Rename `settings/legacy.rs` → `loader.rs` — it's the correct architecture, not legacy
- Crash report dialog: selectable text, copy button
- Toast uses persistent dismissal (pane navigation was clearing it)
diff --git a/apps/desktop/src-tauri/src/crash_reporter/mod.rs b/apps/desktop/src-tauri/src/crash_reporter/mod.rs
@@ -10,6 +10,7 @@ mod symbolicate;
 mod tests;
 
 use crate::config;
+use crate::settings;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
 use std::path::{Path, PathBuf};
@@ -70,8 +71,8 @@ pub fn init<R: tauri::Runtime>(app: &tauri::AppHandle<R>) {
     let crash_path = data_dir.join(CRASH_FILE_NAME);
     let raw_crash_path = data_dir.join(RAW_CRASH_FILE_NAME);
 
-    // Cache active settings from settings.json for crash reports
-    cache_active_settings(&data_dir);
+    // Cache active settings for crash reports, using the same loader as the rest of the app
+    cache_active_settings(app);
 
     // Process any pending crash file from a previous session
     process_pending_crash(&crash_path, &raw_crash_path);
@@ -462,20 +463,18 @@ fn process_pending_crash(crash_json_path: &Path, raw_crash_path: &Path) {
     }
 }
 
-/// Read active settings from settings.json for crash report metadata.
-fn cache_active_settings(data_dir: &Path) {
-    let settings_path = data_dir.join("settings.json");
-    let settings = if let Ok(contents) = std::fs::read_to_string(&settings_path)
-        && let Ok(json) = serde_json::from_str::<serde_json::Value>(&contents)
-    {
-        ActiveSettings {
-            indexing_enabled: json.get("indexing.enabled").and_then(|v| v.as_bool()),
-            ai_provider: json.get("ai.provider").and_then(|v| v.as_str()).map(String::from),
-            mcp_enabled: json.get("developer.mcpEnabled").and_then(|v| v.as_bool()),
-            verbose_logging: json.get("developer.verboseLogging").and_then(|v| v.as_bool()),
-        }
-    } else {
-        ActiveSettings::default()
+/// Cache active settings for crash reports, using the app's settings loader.
+/// This piggybacks on `settings::load_settings` so defaults stay in sync.
+/// Fields that are `None` in the settings struct mean "user hasn't changed this" —
+/// the frontend registry owns the defaults. We pass through `None` as-is; the crash
+/// report consumer can interpret null as "default."
+fn cache_active_settings<R: tauri::Runtime>(app: &tauri::AppHandle<R>) {
+    let s = settings::load_settings(app);
+    let settings = ActiveSettings {
+        indexing_enabled: s.indexing_enabled,
+        ai_provider: s.ai_provider,
+        mcp_enabled: s.developer_mcp_enabled,
+        verbose_logging: s.verbose_logging,
     };
     let _ = CACHED_SETTINGS.set(settings);
 }
@@ -492,7 +491,33 @@ fn uptime_secs() -> f64 {
 }
 
 fn current_thread_count() -> usize {
-    // Read /proc/self/status on Linux, use sysinfo on macOS as best effort
+    #[cfg(target_os = "macos")]
+    {
+        // Mach API: get thread list for the current task, return the count.
+        unsafe extern "C" {
+            fn mach_task_self() -> libc::mach_port_t;
+        }
+        unsafe {
+            let mut thread_list: libc::mach_port_t = 0;
+            let mut thread_count: u32 = 0;
+            let kr = libc::task_threads(
+                mach_task_self(),
+                std::ptr::addr_of_mut!(thread_list) as *mut *mut libc::mach_port_t,
+                std::ptr::addr_of_mut!(thread_count) as *mut u32,
+            );
+            if kr == libc::KERN_SUCCESS {
+                // Deallocate the thread list (we only needed the count)
+                libc::vm_deallocate(
+                    mach_task_self(),
+                    thread_list as libc::vm_address_t,
+                    (thread_count as usize) * size_of::<libc::mach_port_t>(),
+                );
+                thread_count as usize
+            } else {
+                0
+            }
+        }
+    }
     #[cfg(target_os = "linux")]
     {
         if let Ok(status) = std::fs::read_to_string("/proc/self/status") {
@@ -504,10 +529,8 @@ fn current_thread_count() -> usize {
         }
         0
     }
-    #[cfg(not(target_os = "linux"))]
+    #[cfg(not(any(target_os = "macos", target_os = "linux")))]
     {
-        // On macOS, getting thread count without sysinfo is non-trivial.
-        // Return 0 rather than pulling in a heavy dependency just for this.
         0
     }
 }
diff --git a/apps/desktop/src-tauri/src/settings/CLAUDE.md b/apps/desktop/src-tauri/src/settings/CLAUDE.md
@@ -6,8 +6,8 @@ Thin read-only settings loader used during Rust startup. The frontend owns all s
 
 | File | Purpose |
 |---|---|
-| `mod.rs` | Re-exports `load_settings` from `legacy` |
-| `legacy.rs` | `Settings` struct + `load_settings`: reads `settings.json`, falls back to defaults |
+| `mod.rs` | Re-exports `load_settings` from `loader` |
+| `loader.rs` | `Settings` struct + `load_settings`: reads `settings.json`, falls back to defaults |
 
 ## Settings struct
 
@@ -19,6 +19,8 @@ Settings {
     developer_mcp_port: Option<u16>,
     indexing_enabled: Option<bool>,
     crash_reports_enabled: Option<bool>,  // from "updates.crashReports"
+    ai_provider: Option<String>,           // from "ai.provider", for crash reports
+    verbose_logging: Option<bool>,         // from "developer.verboseLogging", for crash reports
 }
 ```
 
@@ -43,7 +45,7 @@ These are top-level keys — the dot is part of the key name, not a nesting sepa
 ## Key patterns
 
 - **One-way read only.** This module never writes. All writes go through the frontend's settings store.
-- Module is named `legacy` because ideally the frontend would push relevant values via IPC at startup rather than requiring Rust to parse the store file directly.
+- Direct file reading is the correct design — multiple backend systems (MCP, indexing, crash reporter) need settings before the frontend loads.
 - `full_disk_access_choice` is marked `#[allow(dead_code)]` — it is persisted by the frontend but the backend takes no action on it.
 - Falls back gracefully: missing file → use `Default`.
 
diff --git a/apps/desktop/src-tauri/src/settings/loader.rs b/apps/desktop/src-tauri/src/settings/loader.rs
@@ -1,7 +1,9 @@
 //! Settings loading from tauri-plugin-store JSON file.
 //!
 //! Reads settings from the settings.json file created by tauri-plugin-store.
-//! Used to initialize app state (menu checkboxes, MCP config) on startup.
+//! The backend reads the file directly at startup so it can configure itself
+//! (MCP server, hidden files, indexing, crash reporter) before the frontend loads.
+//! The frontend owns all writes via tauri-plugin-store; this module is read-only.
 
 use serde::Deserialize;
 use std::fs;
@@ -38,9 +40,15 @@ pub struct Settings {
     #[serde(alias = "updates.crashReports", default)]
     #[allow(
         dead_code,
-        reason = "Crash reporter reads settings.json directly via cache_active_settings()"
+        reason = "Read by frontend; Rust Settings struct preserves it for crash reporter"
     )]
     pub crash_reports_enabled: Option<bool>,
+    #[serde(alias = "ai.provider", default)]
+    #[allow(dead_code, reason = "Included in crash reports for feature correlation")]
+    pub ai_provider: Option<String>,
+    #[serde(alias = "developer.verboseLogging", default)]
+    #[allow(dead_code, reason = "Included in crash reports for feature correlation")]
+    pub verbose_logging: Option<bool>,
 }
 
 fn default_show_hidden() -> bool {
@@ -56,6 +64,8 @@ impl Default for Settings {
             developer_mcp_port: None,
             indexing_enabled: None,
             crash_reports_enabled: None,
+            ai_provider: None,
+            verbose_logging: None,
         }
     }
 }
@@ -100,6 +110,8 @@ fn parse_settings(contents: &str) -> Result<Settings, serde_json::Error> {
     let indexing_enabled = json.get("indexing.enabled").and_then(|v| v.as_bool());
 
     let crash_reports_enabled = json.get("updates.crashReports").and_then(|v| v.as_bool());
+    let ai_provider = json.get("ai.provider").and_then(|v| v.as_str()).map(String::from);
+    let verbose_logging = json.get("developer.verboseLogging").and_then(|v| v.as_bool());
 
     Ok(Settings {
         show_hidden_files,
@@ -108,5 +120,7 @@ fn parse_settings(contents: &str) -> Result<Settings, serde_json::Error> {
         developer_mcp_port,
         indexing_enabled,
         crash_reports_enabled,
+        ai_provider,
+        verbose_logging,
     })
 }
diff --git a/apps/desktop/src-tauri/src/settings/mod.rs b/apps/desktop/src-tauri/src/settings/mod.rs
@@ -1,6 +1,5 @@
-//! Settings module for legacy settings loading.
+//! Settings module — loads settings from tauri-plugin-store's JSON file at startup.
 
-mod legacy;
+mod loader;
 
-// Re-export only what's used externally
-pub use legacy::load_settings;
+pub use loader::load_settings;
diff --git a/apps/desktop/src/lib/crash-reporter/CrashReportDialog.svelte b/apps/desktop/src/lib/crash-reporter/CrashReportDialog.svelte
@@ -18,9 +18,16 @@
     let detailsExpanded = $state(false)
     let alwaysSend = $state(false)
     let sending = $state(false)
+    let copied = $state(false)
 
     const reportJson = $derived(JSON.stringify(report, null, 2))
 
+    async function handleCopy() {
+        await navigator.clipboard.writeText(reportJson)
+        copied = true
+        setTimeout(() => (copied = false), 2000)
+    }
+
     async function handleSend() {
         sending = true
         try {
@@ -79,7 +86,10 @@
 
         {#if detailsExpanded}
             <div class="details-container">
-                <pre class="details-json">{reportJson}</pre>
+                <button class="copy-button" onclick={() => void handleCopy()}>
+                    {copied ? 'Copied' : 'Copy'}
+                </button>
+                <pre class="details-json" style="user-select: text">{reportJson}</pre>
             </div>
         {/if}
 
@@ -142,13 +152,32 @@
     }
 
     .details-container {
+        position: relative;
         margin-bottom: var(--spacing-md);
         border: 1px solid var(--color-border-strong);
         border-radius: var(--radius-md);
         max-height: 200px;
         overflow-y: auto;
     }
 
+    .copy-button {
+        position: sticky;
+        top: var(--spacing-xs);
+        float: right;
+        margin: var(--spacing-xs) var(--spacing-xs) 0 0;
+        padding: var(--spacing-xxs) var(--spacing-sm);
+        font-size: var(--font-size-xs);
+        color: var(--color-text-secondary);
+        border: 1px solid var(--color-border);
+        border-radius: var(--radius-sm);
+        cursor: default;
+        z-index: 1;
+    }
+
+    .copy-button:hover {
+        color: var(--color-text-primary);
+    }
+
     .details-json {
         margin: 0;
         padding: var(--spacing-sm) var(--spacing-md);
diff --git a/apps/desktop/src/routes/(main)/+layout.svelte b/apps/desktop/src/routes/(main)/+layout.svelte
@@ -119,7 +119,7 @@
                     addToast(CrashReportToastContent, {
                         id: 'crash-report-sent',
                         level: 'info',
-                        timeoutMs: 6000,
+                        dismissal: 'persistent',
                     })
                     crashLog.info('Crash report auto-sent')
                 } catch (e) {
