Conversation
…on, offline queue and 30-day log cleanup
Keith-CY
left a comment
There was a problem hiding this comment.
I found two issues that need fixing before merge.
-
[P1] Do not persist bug reports when bug reporting is disabled.
- In
src-tauri/src/bug_report/collector.rs:261the call toqueue::enqueue(...)happens before any settings check. capture()then callsenqueue_event(..., ignore_enabled=false, ...)andprepare_eventdrops sending when disabled, but the queued entry is already written to disk.- Impact: users who explicitly turned off bug reporting still have frontend/panic errors persisted (and may be sent later if reporting is re-enabled).
- Fix: load settings and gate queue enqueue behind the same
!ignore_enabled && settings.enabledcheck, or pass anenabledguard into queue operations.
- In
-
[P2] Queue file operations are not atomic and can lose entries under concurrency.
src-tauri/src/bug_report/queue.rs:137(enqueue) and163(mark_sent) both do read-modify-write cycles with no locking.- Both functions load JSONL into memory, mutate, then rewrite the entire file using
File::create. - Concurrent calls can interleave and overwrite each other (e.g., two
enqueuecalls both read the same snapshot and each writes back, dropping one event). - Fix: serialize queue mutations with a process-wide mutex or file lock around all
enqueue/mark_sent/flush/cleanupwrites.
📊 Test Coverage Report
Coverage measured by |
Code Review — REQUEST_CHANGESCI pending. Two blockers before merge. 🔴 BS 1 —
|
📦 PR Build Artifacts
|
…x OnceLock cache, and single sanitise pass - Add process-level Mutex (pending_lock) to serialise all pending.jsonl/dead_letter.jsonl mutations across enqueue, mark_sent, flush, and cleanup_old_logs — fixes BS1 race between hot-path enqueue and async sender thread mark_sent, and BS2 startup flush vs in-flight sender interleave - Fix CI build failure: cleanup_by_retention turbofish now uses type annotation on the closure parameter instead of ::<T> turbofish (E0107 — method takes 2 generic args) - NBS1: sanitise message/stack_trace once in capture() before branching to disk and network paths; remove redundant sanitize import from queue.rs - NBS2: Linux os_version_string() now uses OnceLock cache (same as macOS); fix OnceLock use import to cover both macos and linux cfg targets - Add mark_sent_is_idempotent_for_unknown_id test
Summary
Closes four gaps identified in the
developbranch bug collection scheme:P0 — Frontend error reporting
capture_frontend_error(Rust + registered ininvoke_handler!)api.captureFrontendError()wrapper inapi.tsApp.tsxhandleUnhandlednow fire-and-forgets a Sentry report in parallel with the existing Guidance LLM flowP1 — OS version detection
src-tauri/src/bug_report/os_info.rsmodule with platform-awareos_version_string()sw_vers -productVersioncached withOnceLockPRETTY_NAMEfrom/etc/os-releaseCurrentBuildNumber→cmd /C ver→$OSenv fallbackOSTYPE/OSenv-var guess incollector.rsP2+P3 — Offline write-ahead queue + 30-day log cleanup
src-tauri/src/bug_report/queue.rswithQueueStoreabstraction<clawpal_dir>/bug_report/:pending.jsonl(max 100),dead_letter.jsonl(max 20, after 3 failed attempts),sent.jsonl(audit log),failures.jsonlcapture()persists to disk first, then attempts immediate send; on success callsmark_sent()BugReportStatsnow exposespersisted_pendinganddead_letter_countlib.rs setup) callscleanup_old_logs()+flush()sent.jsonlandfailures.jsonlentries older than 30 days are pruned on every startupP4 — Structured failure logging
{ts, error}entry tofailures.jsonl(also pruned after 30 days)Tests
queue.rs:enqueue_caps_pending_size,cleanup_prunes_old_failure_logsos_info.rs:linux_pretty_name_parser_handles_quotes(cfg-guarded)