From 9e2dad0b1920ac53bdfa276f246edae6ad318af8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Brandst=C3=A4tter?= Date: Mon, 9 Nov 2026 09:20:40 +0100 Subject: [PATCH] fix(telemetry,client): report background sync ping and use dynamic test timestamps - Report client_ping telemetry upon background sync loop completion in client-tauri spawn_background_sync_loop. - Use dynamic UTC timestamps in backend test_telemetry_api_routes to prevent premature retention pruning. - Add OpenSpec change tracking proposal, delta specs, technical design, and verified tasks. --- backend/tests/test_telemetry.py | 7 ++- client-tauri/src-tauri/src/lib.rs | 5 ++ .../.openspec.yaml | 2 + .../design.md | 48 +++++++++++++++++++ .../proposal.md | 26 ++++++++++ .../specs/central-telemetry/spec.md | 12 +++++ .../tasks.md | 13 +++++ 7 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 openspec/changes/fix-background-sync-ping-and-telemetry-test/.openspec.yaml create mode 100644 openspec/changes/fix-background-sync-ping-and-telemetry-test/design.md create mode 100644 openspec/changes/fix-background-sync-ping-and-telemetry-test/proposal.md create mode 100644 openspec/changes/fix-background-sync-ping-and-telemetry-test/specs/central-telemetry/spec.md create mode 100644 openspec/changes/fix-background-sync-ping-and-telemetry-test/tasks.md diff --git a/backend/tests/test_telemetry.py b/backend/tests/test_telemetry.py index c7e4ddc..369263f 100644 --- a/backend/tests/test_telemetry.py +++ b/backend/tests/test_telemetry.py @@ -145,6 +145,9 @@ def test_telemetry_api_routes(client): headers = {"Authorization": f"Bearer {token}"} # 2. Ingest telemetry batch + now = datetime.now(timezone.utc) + started_at = (now - timedelta(minutes=5)).strftime("%Y-%m-%dT%H:%M:%SZ") + completed_at = (now - timedelta(minutes=4)).strftime("%Y-%m-%dT%H:%M:%SZ") payload = { "events": [ { @@ -164,8 +167,8 @@ def test_telemetry_api_routes(client): "client_type": "desktop-macos", "client_version": "1.3.2", "device_name": "TestMac", - "started_at": "2026-09-16T21:00:00Z", - "completed_at": "2026-09-16T21:01:00Z", + "started_at": started_at, + "completed_at": completed_at, "duration_ms": 60000, "status": "failed", "files_scanned": 100, diff --git a/client-tauri/src-tauri/src/lib.rs b/client-tauri/src-tauri/src/lib.rs index ab8cc7c..3234c1e 100644 --- a/client-tauri/src-tauri/src/lib.rs +++ b/client-tauri/src-tauri/src/lib.rs @@ -710,6 +710,8 @@ fn spawn_background_sync_loop(app: tauri::AppHandle) { client.set_refresh_token(Some(refresh_token)); } let result = sync::run_sync(&mut client, &root); + let sync_ok = result.is_ok(); + let last_sync_at = chrono::Utc::now().to_rfc3339(); match &result { Ok((bytes_downloaded, bytes_uploaded, warning)) => { if let Some(msg) = warning { @@ -731,6 +733,9 @@ fn spawn_background_sync_loop(app: tauri::AppHandle) { update_tray_status(&app_handle, "error", Some(e)); } } + if let Err(e) = client.client_ping(Some(sync_ok), Some(last_sync_at)) { + log::warn!("client_ping failed: {}", e); + } let _ = app_handle.emit("sync-status", sync::get_sync_status_payload()); } } diff --git a/openspec/changes/fix-background-sync-ping-and-telemetry-test/.openspec.yaml b/openspec/changes/fix-background-sync-ping-and-telemetry-test/.openspec.yaml new file mode 100644 index 0000000..ca7e627 --- /dev/null +++ b/openspec/changes/fix-background-sync-ping-and-telemetry-test/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-11-09 diff --git a/openspec/changes/fix-background-sync-ping-and-telemetry-test/design.md b/openspec/changes/fix-background-sync-ping-and-telemetry-test/design.md new file mode 100644 index 0000000..3f47399 --- /dev/null +++ b/openspec/changes/fix-background-sync-ping-and-telemetry-test/design.md @@ -0,0 +1,48 @@ +## Context + +See `proposal.md` for motivation. + +In `client-tauri/src-tauri/src/lib.rs`: +- Manual sync triggered from GUI (`run_sync`) records `sync_ok` and `last_sync_at` and dispatches `client.client_ping(Some(sync_ok), Some(last_sync_at))`. +- Periodic background sync (`spawn_background_sync_loop`) runs the same `sync::run_sync(&mut client, &root)` and updates local UI state, but omitted the call to `client.client_ping`. + +In `backend/tests/test_telemetry.py`: +- `test_telemetry_api_routes` seeded a summary with `"started_at": "2026-09-16T21:00:00Z"`. +- `app.telemetry.service.ingest_telemetry_batch` invokes `prune_telemetry_records`, which deletes any summary older than `now - timedelta(days=30)`. +- When tests execute >30 days past September 16, 2026, the newly ingested summary is immediately deleted during ingestion, causing assertions to fail. + +## Goals / Non-Goals + +**Goals:** +- Unify sync outcome reporting across manual and background synchronization loops in `client-tauri`. +- Make telemetry test timestamps dynamic and resilient to arbitrary future test run dates. +- Keep all fixes strictly scoped, non-breaking, and with zero changes to existing public API schemas. + +**Non-Goals:** +- Changing the retention window duration (30 days remains the server default). +- Adding retry queues for `client_ping` (pings remain best-effort, subsequent sync cycles naturally update server state). + +## Decisions + +### 1. Replicate `client_ping` invocation pattern in `spawn_background_sync_loop` +- **Choice**: Right after `sync::run_sync(&mut client, &root)` completes in `spawn_background_sync_loop`, capture: + ```rust + let sync_ok = result.is_ok(); + let last_sync_at = chrono::Utc::now().to_rfc3339(); + ``` + and invoke: + ```rust + if let Err(e) = client.client_ping(Some(sync_ok), Some(last_sync_at)) { + log::warn!("client_ping failed: {}", e); + } + ``` +- **Rationale**: Matches the behavior of `run_sync` exactly. Any failure to ping is logged at `WARN` level and does not terminate the background loop. + +### 2. Dynamic UTC timestamps in test fixture +- **Choice**: Use Python's `datetime.now(timezone.utc)` to compute `started_at` (5 minutes ago) and `completed_at` (4 minutes ago) formatted as ISO 8601 strings (`.strftime("%Y-%m-%dT%H:%M:%SZ")`). +- **Rationale**: Matches the pattern already established in `test_sync_summary_and_diagnostic_event_crud_and_pruning`, ensuring fixtures never expire regardless of calendar year. + +## Risks / Trade-offs + +- **[Risk]** Network error during client ping in background thread. + - *Mitigation*: Handled as non-fatal with `log::warn!`, identical to manual sync. Next background interval (60s) will attempt sync and ping again. diff --git a/openspec/changes/fix-background-sync-ping-and-telemetry-test/proposal.md b/openspec/changes/fix-background-sync-ping-and-telemetry-test/proposal.md new file mode 100644 index 0000000..41e722c --- /dev/null +++ b/openspec/changes/fix-background-sync-ping-and-telemetry-test/proposal.md @@ -0,0 +1,26 @@ +## Why + +Two concrete bugs currently affect BrandyBox's telemetry and monitoring subsystems: +1. In `client-tauri`, `spawn_background_sync_loop` executes periodic background synchronization runs but completely omits the `client.client_ping(Some(sync_ok), Some(last_sync_at))` call that is performed during manual sync (`run_sync`). Consequently, central server monitoring (`GET /api/admin/clients` and `brandybox-doctor --summary`) displays active clients as disconnected or never synchronized unless manual synchronization is triggered in the GUI. +2. In `backend/tests/test_telemetry.py`, `test_telemetry_api_routes` inserts a hardcoded static timestamp (`2026-09-16T21:00:00Z`). Because `ingest_telemetry_batch` enforces a 30-day retention cleanup via `prune_telemetry_records`, any test run more than 30 days after September 16, 2026 immediately deletes the newly ingested summary, causing the test suite assertion `assert any(s["trace_id"] == "sync-api-test-01" for s in summaries)` to fail. + +Fixing these ensures accurate client observability in the central monitoring dashboard and stabilizes the backend automated test suite. + +## What Changes + +- **Desktop Client Background Ping Telemetry**: Update `spawn_background_sync_loop` in `client-tauri/src-tauri/src/lib.rs` to compute `sync_ok` and `last_sync_at`, invoking `client.client_ping(Some(sync_ok), Some(last_sync_at))` upon completion of each background sync run. +- **Dynamic Telemetry Test Timestamps**: In `backend/tests/test_telemetry.py`, replace static September 2026 timestamp strings with dynamic UTC timestamps relative to `datetime.now(timezone.utc)` so ingested records remain within the 30-day retention window during test assertions. + +## Capabilities + +### New Capabilities + + +### Modified Capabilities +- `central-telemetry`: Clarify requirement that client ping telemetry (`POST /api/clients/ping`) SHALL be dispatched upon completion of both manual and automatic background synchronization cycles. + +## Impact + +- `client-tauri/src-tauri/src/lib.rs`: Background sync loop dispatches `client_ping`. +- `backend/tests/test_telemetry.py`: Test fixtures generate valid dynamic timestamps. +- Admin dashboard (`/api/admin/clients`) and CLI (`brandybox-doctor --summary`): Accurately reflect active client synchronization activity from background daemons. diff --git a/openspec/changes/fix-background-sync-ping-and-telemetry-test/specs/central-telemetry/spec.md b/openspec/changes/fix-background-sync-ping-and-telemetry-test/specs/central-telemetry/spec.md new file mode 100644 index 0000000..a1e06c6 --- /dev/null +++ b/openspec/changes/fix-background-sync-ping-and-telemetry-test/specs/central-telemetry/spec.md @@ -0,0 +1,12 @@ +## ADDED Requirements + +### Requirement: Client ping telemetry upon cycle completion +The client SHALL report its client type, client version, synchronization outcome, and last sync timestamp to the central backend (`POST /api/clients/ping`) upon concluding each synchronization cycle, whether triggered manually by the user or executed automatically in the background daemon loop. + +#### Scenario: Background synchronization cycle concludes +- **WHEN** an automatic periodic background synchronization cycle concludes (whether successful, with warnings, or failed) +- **THEN** the client dispatches a `POST /api/clients/ping` request containing `last_sync_ok` and `last_sync_at` to record active client connection and sync state on the backend. + +#### Scenario: Manual synchronization cycle concludes +- **WHEN** a user-initiated synchronization cycle concludes (whether successful, with warnings, or failed) +- **THEN** the client dispatches a `POST /api/clients/ping` request containing `last_sync_ok` and `last_sync_at` to record active client connection and sync state on the backend. diff --git a/openspec/changes/fix-background-sync-ping-and-telemetry-test/tasks.md b/openspec/changes/fix-background-sync-ping-and-telemetry-test/tasks.md new file mode 100644 index 0000000..2b1d32b --- /dev/null +++ b/openspec/changes/fix-background-sync-ping-and-telemetry-test/tasks.md @@ -0,0 +1,13 @@ +## 1. Backend Telemetry Test Fixture + +- [x] 1.1 Update `test_telemetry_api_routes` in `backend/tests/test_telemetry.py` to use dynamic UTC timestamps for `started_at` and `completed_at` relative to `datetime.now(timezone.utc)`, and verify `pytest backend/tests/test_telemetry.py` passes. + +## 2. Desktop Client Background Sync Telemetry + +- [x] 2.1 In `client-tauri/src-tauri/src/lib.rs`, capture `sync_ok` and `last_sync_at` after `sync::run_sync(&mut client, &root)` and invoke `client.client_ping(Some(sync_ok), Some(last_sync_at))` in `spawn_background_sync_loop`. +- [x] 2.2 Verify client library compiles and tests pass with `cargo test --manifest-path client-tauri/src-tauri/Cargo.toml`. + +## 3. Full System Verification + +- [x] 3.1 Run backend pytest suite with `pytest backend` and verify all tests pass. +- [x] 3.2 Run autonomous sync e2e suite with `pytest tests/e2e/test_run_autonomous_sync.py` and verify all tests pass.