feat: add two-tier opt-in telemetry system

[Miyamura80]

Summary

• Adds a complete CLI-side telemetry system with two opt-in tiers: anonymous (usage stats: test outcomes, duration, error types) and rich (also includes trajectories & screenshots as tarball upload)
• Consent managed via first-run interactive prompt, desktest telemetry subcommand (off/anonymous/rich/status), and DESKTEST_TELEMETRY env var override for CI
• Events are fire-and-forget (5s timeout for stats, 30s for artifact uploads) to a configurable endpoint (DESKTEST_TELEMETRY_URL), failing silently — no user impact if backend is unreachable
• Non-interactive detection (stdin not TTY) skips prompts, periodic nudges every 10 runs for non-rich users

Test plan

• cargo build compiles without errors
• cargo test — all 414 existing + 7 new telemetry unit tests pass
• desktest telemetry status — shows consent level, install ID, run count
• desktest telemetry anonymous / desktest telemetry off — toggles work correctly
• DESKTEST_TELEMETRY=1 desktest telemetry status — env override applies
• First-run prompt on fresh install (delete state file, run desktest run task.json)
• Nudge appears at 10th run (set run_count_since_prompt: 9 in state file)
• Non-interactive: echo "" | desktest run task.json — no prompt (stdin not TTY)
• Fire-and-forget: with anonymous consent, run completes without hang/error even with no backend

🤖 Generated with Claude Code

---

[greptile-apps]

Greptile Summary

This PR introduces a complete two-tier opt-in telemetry system for desktest. Anonymous consent sends lightweight usage-stat events (test outcomes, duration, error type) to /api/events; Rich consent additionally captures a gzipped tarball of the artifacts directory and POSTs it to /api/upload. Consent is managed via a first-run interactive prompt, the desktest telemetry subcommand, and a DESKTEST_TELEMETRY env-var override for CI. The implementation has gone through several review cycles and the major issues flagged in earlier passes (atomic config writes, proper ISO 8601 timestamps, nudge off-by-one, opted-out user nudging, serde(default) on config, atty removal, etc.) have all been addressed.

Key changes:

• src/telemetry.rs (~782 lines): TelemetryClient, TelemetryConfig, consent-check logic, event queue, fire-and-forget flush with timeouts, tarball creation, atomic save_config using write-then-rename
• src/main.rs: telemetry hooks added to Run, Attach, Suite, and Interactive handlers; Interactive correctly guards expected Ctrl+C interrupts from producing spurious error events; provider/model are now populated via record_run_event and the inline suite path
• src/cli.rs: TelemetryAction ValueEnum and Telemetry subcommand added
• One remaining design gap: the /api/upload multipart form only carries install_id with no per-run timestamp or correlation token, making precise backend correlation of artifact uploads to their corresponding events difficult when multiple runs occur close together.

Confidence Score: 4/5

• Safe to merge; the system is fire-and-forget with silent failures so no user-facing regressions are possible, and the core logic is well-tested.
• The PR has gone through extensive review and the substantial issues raised in earlier cycles (atomic writes, correct timestamps, nudge logic, opted-out user handling, serde defaults, TTY detection) are all addressed. The one remaining gap — artifact uploads carrying no run-correlation identifier — is a backend data-quality concern rather than a correctness bug, and the CLI behavior is unaffected either way. All 414 existing tests plus 7 new unit tests pass.
• src/telemetry.rs — the artifact multipart form (around upload_artifacts) could benefit from a per-run correlation identifier.

Important Files Changed

Filename	Overview
src/telemetry.rs	New ~782-line telemetry module: consent management, event queuing, rich artifact upload. Most prior-review issues have been addressed (atomic save, ISO 8601 timestamps, serde defaults, nudge logic, Windows TTY). One remaining design gap: artifact multipart upload carries no run timestamp or correlation ID.
src/main.rs	Integrates telemetry hooks into Run, Attach, Suite, and Interactive command handlers; correctly guards Interactive Ctrl+C from generating spurious error events; provider/model fields now populated; Suite handler uses inline event building instead of the shared helper.
src/cli.rs	Adds TelemetryAction ValueEnum and Telemetry subcommand to the CLI. Clean, no issues.
Cargo.toml	Adds uuid (v4) and multipart/stream features to reqwest. atty dependency removed in favour of std::io::IsTerminal. No issues.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as desktest CLI
    participant FS as telemetry.json (disk)
    participant Events as /api/events
    participant Upload as /api/upload

    User->>CLI: desktest run task.json
    CLI->>FS: load_or_init() — read telemetry.json
    FS-->>CLI: TelemetryConfig (or fresh default)
    CLI->>User: First-run consent prompt (if stdin TTY & not prompted before)
    User-->>CLI: "1" (Anonymous) / "2" (Rich) / "n" (None)
    CLI->>FS: save_config() — persist consent + install_id

    Note over CLI: run_task() executes…

    CLI->>CLI: record_event(test_completed)
    CLI->>User: print result (no wait)

    alt Anonymous or Rich consent
        CLI->>Events: POST /api/events [5s timeout]
        Events-->>CLI: 2xx (or timeout — ignored)
    end

    alt Rich consent + artifacts_dir exists
        CLI->>CLI: create_artifacts_tarball()
        CLI->>Upload: POST /api/upload multipart [30s timeout]
        Upload-->>CLI: 2xx (or timeout — ignored)
    end

    CLI->>User: process exits

Loading

Comments Outside Diff (1)

1. src/telemetry.rs, line 337-342 (link)

   Artifact upload lacks run-correlation identifier

   The multipart form sent to /api/upload only includes install_id. There is no timestamp, session token, or run-specific ID to link this artifact upload to the corresponding test_completed event that was just sent to /api/events.

   In practice a single install_id can have many test runs. The events payload carries per-run timestamp fields, but the artifact upload carries none. The backend must resort to approximate time-window matching to correlate uploads to events, and this fails if two runs happen in close succession.

   Consider adding the current run's event timestamp to the form:

   let form = reqwest::multipart::Form::new()
       .text("install_id", self.config.install_id.clone())
       .text("timestamp", now_iso8601())
       .part("artifacts", part);

   Alternatively, generate a short-lived run_id (e.g., a UUID created at TelemetryClient construction time) and include it in both the event payload and the artifact upload.

_{Reviews (16): Last reviewed commit: "fix: simplify load_config return type, r..." | Re-trigger Greptile}

[Miyamura80]

@greptileai review

[Miyamura80]

@greptileai review

[Miyamura80]

@greptileai review

[Miyamura80]

Addressed all 3 remaining Greptile findings in fe37a01:

1. Stale test fixture (valid) — test_event_serialization timestamp updated to valid ISO 8601
2. Dead home-dir redaction (valid) — Removed the no-op replace on basenames from create_artifacts_tarball
3. #[serde(default)] on TelemetryConfig (valid) — Added for forward-compatible deserialization

@greptileai review

[Miyamura80]

Addressed 2 of 3 remaining Greptile findings in 572f767:

1. Interactive not wired (valid) — Added record_event, set_artifacts_dir, and flush to Interactive handler, matching Run/Suite/Attach pattern
2. Ephemeral install_id under env override (valid) — Fresh config is now persisted on first load, so install_id is stable across CI runs
3. Windows HOME (false positive) — desktest is a Linux desktop testing tool (Docker + XFCE + Xvfb). Windows is not a supported platform. The tool requires Docker with Linux containers and X11.

@greptileai review

[Miyamura80]

Fixed the last remaining issue in 25a9242:

Interactive Ctrl+C classified as error (valid) — record_run_event is now skipped for expected Ctrl+C interrupts in interactive mode (no --step, no --validate-only), avoiding inflated error metrics.

@greptileai review

[Miyamura80]

Fixed both remaining issues in d4314ae:

1. Env-override consent leaks to disk (valid) — save_config now runs before the env override is applied, so only install_id is persisted, not the overridden consent level
2. Unreachable None arm in show_nudge (valid) — Removed dead code; only Anonymous users can reach show_nudge

@greptileai review

[Miyamura80]

Addressed Greptile round-8 findings in 9b0138a:

1. P1: Silent config-corruption reset (valid) — load_config() now distinguishes "file not found" from "parse failure" and warns the user before resetting
2. P2: Unnecessary disk writes for Rich users (valid) — check_consent() now skips incrementing and persisting run_count for Rich users (they can never be nudged)
3. P2: Event type naming asymmetry (invalid) — suite_completed vs test_completed is intentional: suites and individual tests are semantically different events. The backend should distinguish them for aggregate vs per-test analytics.

@greptileai review

[Miyamura80]

Addressed remaining Greptile round-9 findings in 93c8207:

1. P1: Silent config-corruption reset (valid) — Already fixed in 9b0138a, load_config now warns before resetting
2. P2: provider/model fields never populated (valid) — All telemetry events now include provider and model from Config
3. P2: Unnecessary disk writes for Rich (valid) — Already fixed in 9b0138a
4. P2: Event type naming asymmetry (invalid) — suite_completed vs test_completed is intentional; suites and individual tests are semantically different

@greptileai review

[Miyamura80]

Fixed both remaining issues in 1d303c7:

1. Silent fresh-install on I/O read errors (valid) — load_config now distinguishes ENOENT from other I/O errors (e.g. permission denied) and warns before resetting
2. Orphaned artifact upload on Interactive Ctrl+C (valid) — set_artifacts_dir and flush are now skipped when Ctrl+C is an expected interrupt, preventing uploads with no correlated event

@greptileai review

[Miyamura80]

Fixed all 3 remaining style issues in b10fcb7:

1. Silent empty tarball (valid) — Now logs a debug message and returns early when read_dir fails
2. Unnecessary disk write for opted-out users (valid) — Skip increment+save for None+prompted_at users (same as Rich)
3. Non-atomic save_config (valid) — Now uses write-to-tmp-then-rename pattern

@greptileai review

[Miyamura80]

Fixed both issues in 0e55999:

1. Suite --artifacts-dir contradiction (valid) — Removed telemetry artifacts_dir setting for Suite, consistent with the user-facing warning that --artifacts-dir is ignored for suite runs
2. Dead next_count > 0 guard (valid) — Removed (u32 + 1 is always >= 1)

@greptileai review

[Miyamura80]

Fixed 2 of 3 issues from round 13 in f92c6a8:

1. Case-sensitive extension matching (valid) — Extensions are now lowercased before comparison, so .PNG/.JSONL are included
2. Suite provider/model mismatch (invalid) — The provider/model represents the CLI-level config passed to all tasks. Per-task config overrides are not a feature of desktest; all tasks in a suite use the same config.
3. Unconditional Client::new() (valid) — Client is now only constructed when there are events to send or artifacts to upload

@greptileai review

[Miyamura80]

Fixed remaining style issues in c733526:

1. Free-form String action (valid) — Replaced with clap::ValueEnum enum; clap now validates at parse time and shows possible values on error
2. Stale .json.tmp on rename failure (valid) — tmp file is now cleaned up if rename fails
3. Blocking stdin.read_line in async (invalid) — This runs at startup before any async work is spawned; the Greptile summary already acknowledges "safe today because no other tasks are running at that point"

@greptileai review

[Miyamura80]

Fixed 2 of 3 remaining observations in 7a08bf9:

1. Redundant was_corrupt bool (valid) — Simplified load_config to return Option; warnings printed inline
2. Redundant show_nudge guard (valid) — Removed; comment documents the invariant instead
3. error_category only exit codes (invalid) — Exit codes map cleanly to AppError variants (2=config, 3=infra, 4=agent); this is the right abstraction level for backend analytics without leaking internal error details

@greptileai review