fix(client): record sync summary and flush telemetry on delete failure

client-tauri/src-tauri/src/config.rs

@@ -311,6 +311,9 @@ pub fn get_device_name() -> String {
         .unwrap_or_else(|_| "Desktop-Client".to_string())
 }
 
+#[cfg(test)]
+pub static ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());
+
 #[cfg(test)]
 mod tests {
     use super::*;

client-tauri/src-tauri/src/credentials.rs

@@ -183,11 +183,10 @@ pub fn clear_stored() {
 #[cfg(test)]
 mod tests {
     use super::*;
-    static TEST_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());
 
     #[test]
     fn test_store_and_get() {
-        let _lock = TEST_MUTEX.lock().unwrap();
+        let _lock = crate::config::ENV_MUTEX.lock().unwrap();
         let temp_dir = std::env::temp_dir().join(format!("brandybox_test_{}", uuid::Uuid::new_v4()));
         std::env::set_var("BRANDYBOX_CONFIG_DIR", &temp_dir);
         clear_stored();
@@ -216,7 +215,7 @@ mod tests {
 
     #[test]
     fn test_get_stored_expired_token() {
-        let _lock = TEST_MUTEX.lock().unwrap();
+        let _lock = crate::config::ENV_MUTEX.lock().unwrap();
         let temp_dir = std::env::temp_dir().join(format!("brandybox_test_{}", uuid::Uuid::new_v4()));
         std::env::set_var("BRANDYBOX_CONFIG_DIR", &temp_dir);
         clear_stored();

client-tauri/src-tauri/src/sync.rs

@@ -365,6 +365,7 @@ pub fn run_sync(client: &mut ApiClient, local_root: &Path) -> Result<(u64, u64,
         set_progress("delete_server", done, total_work);
         if let Err(e) = client.delete_file(path) {
             let err_code = classify_error(&e);
+            let err_msg = format!("Delete server {}: {}", path, e);
             enqueue_diagnostic_event(crate::api::DiagnosticEventPayload {
                 trace_id: Some(trace_id.clone()),
                 created_at: Some(chrono::Utc::now().to_rfc3339()),
@@ -373,10 +374,27 @@ pub fn run_sync(client: &mut ApiClient, local_root: &Path) -> Result<(u64, u64,
                 level: "ERROR".to_string(),
                 category: "sync.delete".to_string(),
                 error_code: err_code.to_string(),
-                message: format!("Delete server {}: {}", path, e),
+                message: err_msg.clone(),
                 context_json: Some(serde_json::json!({ "path": path }).to_string()),
             });
-            return Err(format!("Delete server {}: {}", path, e));
+            enqueue_sync_summary(crate::api::SyncSummaryPayload {
+                trace_id: trace_id.clone(),
+                client_type: config::get_client_type().to_string(),
+                client_version: env!("CARGO_PKG_VERSION").to_string(),
+                device_name: config::get_device_name(),
+                started_at: started_at.clone(),
+                completed_at: Some(chrono::Utc::now().to_rfc3339()),
+                duration_ms: start_instant.elapsed().as_millis() as i64,
+                status: "failed".to_string(),
+                files_scanned: (local_list.len() + remote_list.len()) as i64,
+                files_uploaded: 0,
+                files_downloaded: 0,
+                failure_count: 1,
+                bytes_transferred: 0,
+                error_summary_json: Some(serde_json::json!({ "error": err_msg }).to_string()),
+            });
+            let _ = flush_telemetry_queue(client);
+            return Err(err_msg);
         }
         state.file_hashes.remove(path);
         done += 1;
@@ -1108,5 +1126,78 @@ mod tests {
         state.file_hashes.remove("deleted.txt");
         assert_eq!(state.file_hashes.len(), 0);
     }
+
+    #[test]
+    fn test_sync_delete_failure_records_summary_and_flushes_telemetry() {
+        let _lock = crate::config::ENV_MUTEX.lock().unwrap();
+        use std::io::{Read, Write};
+        use std::net::TcpListener;
+        use std::sync::atomic::{AtomicBool, Ordering};
+        use std::sync::Arc;
+
+        let temp_dir = std::env::temp_dir().join(format!("test_sync_del_tel_{}", uuid::Uuid::new_v4()));
+        let _ = std::fs::create_dir_all(&temp_dir);
+        let sync_dir = temp_dir.join("sync_folder");
+        let _ = std::fs::create_dir_all(&sync_dir);
+        let config_dir = temp_dir.join("config");
+        let _ = std::fs::create_dir_all(&config_dir);
+
+        std::env::set_var("BRANDYBOX_CONFIG_DIR", config_dir.to_str().unwrap());
+
+        let mut initial_state = SyncStateFile::default();
+        initial_state.paths.push("to_remove.txt".to_string());
+        save_sync_state(&initial_state);
+
+        let listener = TcpListener::bind("127.0.0.1:0").unwrap();
+        let port = listener.local_addr().unwrap().port();
+        let base_url = format!("http://127.0.0.1:{}", port);
+
+        let telemetry_flushed = Arc::new(AtomicBool::new(false));
+        let telemetry_flushed_clone = telemetry_flushed.clone();
+
+        let server_thread = std::thread::spawn(move || {
+            for stream in listener.incoming() {
+                if let Ok(mut s) = stream {
+                    let mut buf = [0u8; 2048];
+                    let n = s.read(&mut buf).unwrap_or(0);
+                    let req = String::from_utf8_lossy(&buf[..n]);
+                    if req.starts_with("GET /api/files/list") {
+                        let body = r#"[{"path":"to_remove.txt","mtime":100.0,"size":10,"hash":"abc"}]"#;
+                        let resp = format!(
+                            "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}",
+                            body.len(),
+                            body
+                        );
+                        let _ = s.write_all(resp.as_bytes());
+                    } else if req.starts_with("DELETE /api/files/delete") {
+                        let resp = "HTTP/1.1 403 Forbidden\r\nContent-Length: 0\r\nConnection: close\r\n\r\n";
+                        let _ = s.write_all(resp.as_bytes());
+                    } else if req.starts_with("POST /api/telemetry/events") {
+                        telemetry_flushed_clone.store(true, Ordering::SeqCst);
+                        // Return 500 so queue stays saved on disk for inspection
+                        let resp = "HTTP/1.1 500 Internal Server Error\r\nContent-Length: 0\r\nConnection: close\r\n\r\n";
+                        let _ = s.write_all(resp.as_bytes());
+                        break;
+                    }
+                }
+            }
+        });
+
+        let mut client = ApiClient::new(base_url);
+        client.set_access_token(Some("dummy_token".to_string()));
+
+        let res = run_sync(&mut client, &sync_dir);
+        assert!(res.is_err(), "run_sync should return Err on delete failure");
+
+        let queue = load_telemetry_queue();
+        assert_eq!(queue.summaries.len(), 1, "Expected 1 SyncSummaryPayload enqueued on delete failure");
+        assert_eq!(queue.summaries[0].status, "failed");
+        assert_eq!(queue.summaries[0].failure_count, 1);
+        assert!(telemetry_flushed.load(Ordering::SeqCst), "Expected flush_telemetry_queue to be called");
+
+        std::env::remove_var("BRANDYBOX_CONFIG_DIR");
+        let _ = server_thread.join();
+        let _ = std::fs::remove_dir_all(&temp_dir);
+    }
 }
 

openspec/changes/fix-sync-delete-failure-telemetry-summary/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-11-03

openspec/changes/fix-sync-delete-failure-telemetry-summary/design.md

@@ -0,0 +1,54 @@
+## Context
+
+In Brandy Box desktop client, `run_sync` in `client-tauri/src-tauri/src/sync.rs` orchestrates the two-way sync loop between the local sync directory and the backend server.
+Telemetry tracking is split into two models:
+1. `DiagnosticEvent`: Granular diagnostic events detailing specific errors (e.g. `sync.list`, `sync.delete`, `sync.download`, `sync.upload`).
+2. `SyncSummary`: Per-sync-cycle overview documenting `status` (`"ok"`, `"warning"`, `"failed"`), duration, file counts scanned/transferred, failure count, and error summary.
+
+During `run_sync`:
+- If `client.list_files()` fails (lines 283–316), both a `DiagnosticEvent` and a `SyncSummary` with `status: "failed"` are enqueued, and `flush_telemetry_queue(client)` is invoked before returning `Err(e)`.
+- If sync finishes without early error (lines 728–744), a `SyncSummary` with `status: "ok"` or `"warning"` is enqueued and `flush_telemetry_queue(client)` is invoked.
+- However, if `client.delete_file(path)` fails during remote deletion (lines 364–380), only a `DiagnosticEvent` was queued. `enqueue_sync_summary` and `flush_telemetry_queue` were omitted before returning `Err`.
+
+## Goals / Non-Goals
+
+**Goals:**
+- Ensure that any early exit from `run_sync` due to remote deletion failure records a `SyncSummaryPayload` with `status: "failed"` and `failure_count: 1`.
+- Ensure `flush_telemetry_queue(client)` is invoked prior to returning `Err` so telemetry is dispatched immediately or saved to the offline buffer.
+- Add regression testing to verify that delete failure generates the expected summary and triggers telemetry queue flushing.
+
+**Non-Goals:**
+- Changing retry semantics of `ApiClient::delete_file`.
+- Modifying how callers in `lib.rs` handle the returned `Err`.
+
+## Decisions
+
+### Decision 1: Populate `SyncSummaryPayload` on `delete_file` Failure
+When `client.delete_file(path)` returns `Err(e)`:
+- Create error message: `let err_msg = format!("Delete server {}: {}", path, e);`
+- Enqueue diagnostic event (category `"sync.delete"`, error code from `classify_error(&e)`).
+- Enqueue sync summary:
+  - `trace_id`: `trace_id.clone()`
+  - `client_type`: `config::get_client_type().to_string()`
+  - `client_version`: `env!("CARGO_PKG_VERSION").to_string()`
+  - `device_name`: `config::get_device_name()`
+  - `started_at`: `started_at.clone()`
+  - `completed_at`: `Some(chrono::Utc::now().to_rfc3339())`
+  - `duration_ms`: `start_instant.elapsed().as_millis() as i64`
+  - `status`: `"failed".to_string()`
+  - `files_scanned`: `(local_list.len() + remote_list.len()) as i64`
+  - `files_uploaded`: 0
+  - `files_downloaded`: 0
+  - `failure_count`: 1
+  - `bytes_transferred`: 0
+  - `error_summary_json`: `Some(serde_json::json!({ "error": err_msg }).to_string())`
+- Call `let _ = flush_telemetry_queue(client);`
+- Return `Err(err_msg)`
+
+*Rationale*:
+Mirrors the pattern established in `list_files` error handling and ensures symmetry across failure exit points.
+
+## Risks / Trade-offs
+
+- **[Risk]** If backend is unreachable, `flush_telemetry_queue` will fail.
+  - **Mitigation**: `flush_telemetry_queue` already handles network failure gracefully by retaining items in the offline queue file on disk (`config::get_telemetry_queue_path()`) and logging a warning. The next sync attempt flushes them.

openspec/changes/fix-sync-delete-failure-telemetry-summary/proposal.md

@@ -0,0 +1,27 @@
+## Why
+
+In `client-tauri/src-tauri/src/sync.rs`, the desktop sync engine synchronizes file deletions by identifying files removed locally and deleting them from the remote server via `client.delete_file(path)`. If `client.delete_file(path)` returns an error (e.g. HTTP 403, 500, network disconnect), the loop records a `DiagnosticEventPayload` and immediately terminates `run_sync` by returning `Err(format!("Delete server {}: {}", path, e))`.
+
+However, unlike the `list_files` error handler (which properly enqueues a `SyncSummaryPayload` with status `"failed"` and flushes the telemetry queue via `flush_telemetry_queue`), the `delete_file` error path:
+1. Omits calling `enqueue_sync_summary`, meaning no `SyncSummary` entry is ever created or persisted for this failed synchronization run.
+2. Omits calling `flush_telemetry_queue(client)`, leaving the queued `DiagnosticEvent` unflushed in the offline queue file.
+3. Leaves server administrators and diagnostics tools (`brandybox doctor`) completely blind to the failed sync run in aggregate metrics and summary tables.
+
+## What Changes
+
+- In `client-tauri/src-tauri/src/sync.rs`, when `client.delete_file(path)` fails:
+  - Enqueue a `SyncSummaryPayload` with `status: "failed"`, `failure_count: 1`, elapsed `duration_ms`, `files_scanned: (local_list.len() + remote_list.len()) as i64`, and an `error_summary_json` detailing the delete failure.
+  - Call `flush_telemetry_queue(client)` before returning the error so that both the diagnostic event and the sync summary are sent to the backend immediately (or retained in the offline buffer if offline).
+- Add regression unit test in `client-tauri/src-tauri/src/sync.rs` asserting that when server file deletion fails during synchronization, a `"failed"` `SyncSummaryPayload` is queued and telemetry flushing is triggered.
+
+## Capabilities
+
+### Modified Capabilities
+- `desktop-client`: Require the desktop client synchronization engine to generate a failed `SyncSummaryPayload` and flush telemetry when a remote file deletion fails during a synchronization cycle.
+
+## Impact
+
+- **Affected code**: `client-tauri/src-tauri/src/sync.rs`
+- **APIs**: No changes to API schemas; existing `/api/telemetry/events` payload is properly populated.
+- **Dependencies**: No new dependencies.
+- **Behavior**: Synchronization runs that fail during remote file deletion now reliably emit `SyncSummary` records with status `"failed"`, allowing server telemetry and `brandybox doctor` to reflect sync failures accurately.

openspec/changes/fix-sync-delete-failure-telemetry-summary/specs/desktop-client/spec.md

@@ -0,0 +1,8 @@
+## ADDED Requirements
+
+### Requirement: Sync Summary Emission on Remote Deletion Failure
+When the desktop client synchronization engine encounters an error while deleting a remote file on the server, it SHALL record a `SyncSummary` event with status `"failed"`, document the failure count, error summary, and scan metrics, and attempt to flush the telemetry queue before returning the error.
+
+#### Scenario: Server file deletion returns an error
+- **WHEN** a local file was deleted and scheduled for remote deletion via `client.delete_file(path)`, but the server request fails with an error (e.g. HTTP 403, 500, or network disconnect)
+- **THEN** the sync engine records a `DiagnosticEvent` with category `"sync.delete"`, records a `SyncSummary` with `status: "failed"` and `failure_count: 1`, flushes the telemetry queue, and returns the error to the caller.

openspec/changes/fix-sync-delete-failure-telemetry-summary/tasks.md

@@ -0,0 +1,14 @@
+## 1. Unit Testing and Reproduction
+
+- [x] 1.1 Add unit test in `client-tauri/src-tauri/src/sync.rs` asserting that when server file deletion fails during `run_sync`, a `SyncSummary` with `status: "failed"` is enqueued and telemetry flush is executed.
+- [x] 1.2 Verify the reproduction test fails or demonstrates missing summary before the fix.
+
+## 2. Desktop Client Implementation
+
+- [x] 2.1 Update `run_sync` in `client-tauri/src-tauri/src/sync.rs` on `client.delete_file` error to call `enqueue_sync_summary` with `status: "failed"` and invoke `flush_telemetry_queue(client)`.
+- [x] 2.2 Verify that all tests pass.
+
+## 3. Verification and Quality Assurance
+
+- [x] 3.1 Run `cargo test` in `client-tauri/src-tauri` and verify all tests pass.
+- [x] 3.2 Run repo-wide QA checks and confirm no regressions.