From 4ed831d9c3eec969e9965e693d87a75c5e5c79aa Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 29 Jun 2026 18:38:56 -0400 Subject: [PATCH 1/5] feat(server/diagnostics): device debug-reporting ingest + admin timeline + retention (M9) New diagnostic_events table + per-account users.debug_mode_enabled flag. When an account's flag is on, its client(s) POST a batch timeseries of connectivity / UPnP-sync / power / lifecycle events to /api/diagnostics (no-op 204 when off, kind whitelist mirrors the CHECK constraint). Admin surface: GET /api/admin/diagnostics (optional account/device/kind/ time-window filters, RFC3339-or-epoch-ms, export-sized paging) + a /diagnostics/devices overview + PUT /api/admin/users/{id}/debug-mode to flip an account remotely while a bug is live. debug_mode_enabled is now exposed on /api/me (client gate) and the admin user views. Retention: a 30-day gc-worker sweep (GcPruneDiagnostics), keyed on the server clock so a skewed device clock can't keep rows alive. Refs Scribe M9 (#119), tasks #1172 #1173. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- internal/api/admin_users.go | 5 + internal/api/api.go | 11 + internal/api/diagnostics.go | 345 ++++++++++++++++++ internal/api/me_profile.go | 15 +- internal/db/dbq/diagnostics.sql.go | 221 +++++++++++ internal/db/dbq/models.go | 13 + internal/db/dbq/users.sql.go | 76 +++- .../db/migrations/0036_diagnostics.down.sql | 2 + .../db/migrations/0036_diagnostics.up.sql | 54 +++ internal/db/queries/diagnostics.sql | 66 ++++ internal/db/queries/users.sql | 12 +- internal/gc/worker.go | 2 + 12 files changed, 804 insertions(+), 18 deletions(-) create mode 100644 internal/api/diagnostics.go create mode 100644 internal/db/dbq/diagnostics.sql.go create mode 100644 internal/db/migrations/0036_diagnostics.down.sql create mode 100644 internal/db/migrations/0036_diagnostics.up.sql create mode 100644 internal/db/queries/diagnostics.sql diff --git a/internal/api/admin_users.go b/internal/api/admin_users.go index 550014f8..57eb8010 100644 --- a/internal/api/admin_users.go +++ b/internal/api/admin_users.go @@ -22,6 +22,7 @@ type adminUserView struct { DisplayName *string `json:"display_name"` IsAdmin bool `json:"is_admin"` AutoApproveRequests bool `json:"auto_approve_requests"` + DebugModeEnabled bool `json:"debug_mode_enabled"` CreatedAt string `json:"created_at"` } @@ -44,6 +45,7 @@ func (h *handlers) handleAdminListUsers(w http.ResponseWriter, r *http.Request) DisplayName: row.DisplayName, IsAdmin: row.IsAdmin, AutoApproveRequests: row.AutoApproveRequests, + DebugModeEnabled: row.DebugModeEnabled, CreatedAt: row.CreatedAt.Time.UTC().Format(time.RFC3339), } out = append(out, v) @@ -120,6 +122,7 @@ func (h *handlers) handleUpdateUserAdmin(w http.ResponseWriter, r *http.Request) DisplayName: updated.DisplayName, IsAdmin: updated.IsAdmin, AutoApproveRequests: updated.AutoApproveRequests, + DebugModeEnabled: updated.DebugModeEnabled, CreatedAt: updated.CreatedAt.Time.UTC().Format(time.RFC3339), }) } @@ -189,6 +192,7 @@ func (h *handlers) handleAdminCreateUser(w http.ResponseWriter, r *http.Request) DisplayName: user.DisplayName, IsAdmin: user.IsAdmin, AutoApproveRequests: user.AutoApproveRequests, + DebugModeEnabled: user.DebugModeEnabled, CreatedAt: user.CreatedAt.Time.UTC().Format(time.RFC3339), }) } @@ -329,6 +333,7 @@ func (h *handlers) handleAdminAutoApproveToggle(w http.ResponseWriter, r *http.R DisplayName: updated.DisplayName, IsAdmin: updated.IsAdmin, AutoApproveRequests: updated.AutoApproveRequests, + DebugModeEnabled: updated.DebugModeEnabled, CreatedAt: updated.CreatedAt.Time.UTC().Format(time.RFC3339), }) } diff --git a/internal/api/api.go b/internal/api/api.go index 4639cca2..5a7e4bfe 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -135,6 +135,12 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev // load failures). Admin-only inbox; any user can report. authed.Post("/playback-errors", h.handleReportPlaybackError) + // Device diagnostics ingest (M9). Any signed-in user can + // POST a batch, but events are only stored when the + // account's debug_mode_enabled flag is on (handler no-ops + // otherwise). Admin views live under /admin/diagnostics. + authed.Post("/diagnostics", h.handleReportDiagnostics) + // Self-hosted in-app update channel (#397). Auth-gated to // prevent anonymous bandwidth abuse on the APK stream; // /apk additionally per-user rate-limited. @@ -181,6 +187,11 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Delete("/users/{id}", h.handleAdminDeleteUser) admin.Post("/users/{id}/reset-password", h.handleAdminResetPassword) admin.Put("/users/{id}/auto-approve", h.handleAdminAutoApproveToggle) + admin.Put("/users/{id}/debug-mode", h.handleAdminDebugModeToggle) + + // Device diagnostics timeline + device overview (M9). + admin.Get("/diagnostics", h.handleListAdminDiagnostics) + admin.Get("/diagnostics/devices", h.handleListAdminDiagnosticDevices) admin.Get("/cover-sources", h.handleListCoverSources) admin.Patch("/cover-sources/{provider_id}", h.handleUpdateCoverSource) diff --git a/internal/api/diagnostics.go b/internal/api/diagnostics.go new file mode 100644 index 00000000..23dbfe92 --- /dev/null +++ b/internal/api/diagnostics.go @@ -0,0 +1,345 @@ +package api + +import ( + "encoding/json" + "net/http" + "strconv" + "strings" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// validDiagnosticKinds is the COARSE category whitelist. Mirrors the +// CHECK constraint in migration 0036 — keep both in sync (per the +// enum-CHECK-whitelist rule). The finer event discriminator + all +// event-specific fields live inside each event's payload, so new event +// variants do NOT require a new category here. +var validDiagnosticKinds = map[string]struct{}{ + "connectivity": {}, + "upnp_sync": {}, + "power": {}, + "lifecycle": {}, + "heartbeat": {}, + "http": {}, +} + +// maxDiagnosticBatch caps a single ingest call. The client buffers and +// flushes in batches of ~100; 500 leaves generous headroom while +// bounding a misbehaving client's per-request work. +const maxDiagnosticBatch = 500 + +// Diagnostics list paging. Larger than the generic parsePaging caps +// because the admin timeline is meant to be exported as a slice and +// handed off for analysis — a heartbeat-dense window has many rows. +const ( + defaultDiagnosticPageSize = 500 + maxDiagnosticPageSize = 5000 +) + +// diagnosticEventIn is one event in a POST /api/diagnostics batch. +// OccurredAt is the client device clock as epoch milliseconds (may be +// skewed — the server also stamps received_at). Payload is opaque JSON +// carrying the event's sub-type + fields. +type diagnosticEventIn struct { + Kind string `json:"kind"` + OccurredAt int64 `json:"occurred_at"` + Payload json.RawMessage `json:"payload"` +} + +// reportDiagnosticsRequest is the body of POST /api/diagnostics — one +// batch from one device. +type reportDiagnosticsRequest struct { + ClientID string `json:"client_id"` + AppVersion *string `json:"app_version,omitempty"` + OsVersion *string `json:"os_version,omitempty"` + Events []diagnosticEventIn `json:"events"` +} + +// handleReportDiagnostics implements POST /api/diagnostics. Any signed-in +// user can call it, but events are only stored when the account's +// debug_mode_enabled flag is on; otherwise the call is a 204 no-op so the +// client can safely drop the batch (it gates locally on /api/me, this is +// the race-safe backstop for "admin just disabled debug"). +func (h *handlers) handleReportDiagnostics(w http.ResponseWriter, r *http.Request) { + user, ok := requireUser(w, r) + if !ok { + return + } + if !user.DebugModeEnabled { + w.WriteHeader(http.StatusNoContent) + return + } + var req reportDiagnosticsRequest + if !decodeBody(w, r, &req) { + return + } + if strings.TrimSpace(req.ClientID) == "" { + writeErr(w, apierror.BadRequest("bad_request", "client_id required")) + return + } + if len(req.Events) == 0 { + writeJSON(w, http.StatusAccepted, map[string]int{"accepted": 0}) + return + } + if len(req.Events) > maxDiagnosticBatch { + writeErr(w, apierror.BadRequest("batch_too_large", "too many events in one batch")) + return + } + for i := range req.Events { + if _, valid := validDiagnosticKinds[req.Events[i].Kind]; !valid { + writeErr(w, apierror.BadRequest("invalid_kind", "unknown diagnostic kind")) + return + } + } + + err := pgx.BeginFunc(r.Context(), h.pool, func(tx pgx.Tx) error { + q := dbq.New(tx) + for i := range req.Events { + ev := req.Events[i] + payload := ev.Payload + if len(payload) == 0 { + payload = json.RawMessage("{}") + } + if _, err := q.InsertDiagnosticEvent(r.Context(), dbq.InsertDiagnosticEventParams{ + UserID: user.ID, + ClientID: req.ClientID, + AppVersion: req.AppVersion, + OsVersion: req.OsVersion, + Kind: ev.Kind, + Payload: payload, + OccurredAt: pgtype.Timestamptz{Time: time.UnixMilli(ev.OccurredAt).UTC(), Valid: true}, + }); err != nil { + return err + } + } + return nil + }) + if err != nil { + writeErrWithLog(w, h.logger, "diagnostics: insert batch failed", apierror.Internal(err)) + return + } + writeJSON(w, http.StatusAccepted, map[string]int{"accepted": len(req.Events)}) +} + +// adminDiagnosticView is one row in the admin timeline response. Payload +// is passed through verbatim as raw JSON so the SPA can render / export +// the structured event without the server re-shaping it. +type adminDiagnosticView struct { + ID string `json:"id"` + UserID string `json:"user_id"` + Username string `json:"username"` + ClientID string `json:"client_id"` + AppVersion *string `json:"app_version,omitempty"` + OsVersion *string `json:"os_version,omitempty"` + Kind string `json:"kind"` + Payload json.RawMessage `json:"payload"` + OccurredAt string `json:"occurred_at"` + ReceivedAt string `json:"received_at"` +} + +// handleListAdminDiagnostics implements GET /api/admin/diagnostics with +// optional filters user_id, client_id, kind, from, to (RFC3339 or epoch +// millis) plus limit/offset. Newest-first; the SPA reverses for a +// chronological timeline + export. +func (h *handlers) handleListAdminDiagnostics(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + params, ok := h.buildDiagnosticsFilter(w, q) + if !ok { + return + } + rows, err := dbq.New(h.pool).ListAdminDiagnostics(r.Context(), params) + if err != nil { + writeErrWithLog(w, h.logger, "admin: list diagnostics failed", apierror.Internal(err)) + return + } + out := make([]adminDiagnosticView, 0, len(rows)) + for _, row := range rows { + out = append(out, adminDiagnosticView{ + ID: uuidToString(row.ID), + UserID: uuidToString(row.UserID), + Username: row.Username, + ClientID: row.ClientID, + AppVersion: row.AppVersion, + OsVersion: row.OsVersion, + Kind: row.Kind, + Payload: json.RawMessage(row.Payload), + OccurredAt: formatTimestamp(row.OccurredAt), + ReceivedAt: formatTimestamp(row.ReceivedAt), + }) + } + writeJSON(w, http.StatusOK, out) +} + +// buildDiagnosticsFilter parses the query string into a query param +// struct, writing a 400 and returning ok=false on a malformed time. +func (h *handlers) buildDiagnosticsFilter( + w http.ResponseWriter, q map[string][]string, +) (dbq.ListAdminDiagnosticsParams, bool) { + get := func(k string) string { + if v, present := q[k]; present && len(v) > 0 { + return v[0] + } + return "" + } + limit, offset, err := parseDiagPaging(get("limit"), get("offset")) + if err != nil { + writeErr(w, apierror.BadRequest("bad_request", "invalid limit or offset")) + return dbq.ListAdminDiagnosticsParams{}, false + } + from, okFrom := parseDiagTime(get("from")) + to, okTo := parseDiagTime(get("to")) + if !okFrom || !okTo { + writeErr(w, apierror.BadRequest("bad_request", "invalid from/to timestamp")) + return dbq.ListAdminDiagnosticsParams{}, false + } + params := dbq.ListAdminDiagnosticsParams{ + ClientID: optionalQuery(get("client_id")), + Kind: optionalQuery(get("kind")), + FromTs: from, + ToTs: to, + Off: int32(offset), + Lim: int32(limit), + } + if uid, valid := parseUUID(get("user_id")); valid { + params.UserID = uid + } + return params, true +} + +// adminDiagnosticDeviceView is one device in the "who is reporting" +// overview / device filter dropdown. +type adminDiagnosticDeviceView struct { + ClientID string `json:"client_id"` + UserID string `json:"user_id"` + Username string `json:"username"` + AppVersion string `json:"app_version"` + OsVersion string `json:"os_version"` + LastSeen string `json:"last_seen"` + EventCount int64 `json:"event_count"` +} + +// handleListAdminDiagnosticDevices implements GET +// /api/admin/diagnostics/devices?user_id=. Lists distinct reporting +// devices so the admin UI can populate its device filter. +func (h *handlers) handleListAdminDiagnosticDevices(w http.ResponseWriter, r *http.Request) { + var userID pgtype.UUID + if uid, valid := parseUUID(r.URL.Query().Get("user_id")); valid { + userID = uid + } + rows, err := dbq.New(h.pool).ListDiagnosticDevices(r.Context(), userID) + if err != nil { + writeErrWithLog(w, h.logger, "admin: list diagnostic devices failed", apierror.Internal(err)) + return + } + out := make([]adminDiagnosticDeviceView, 0, len(rows)) + for _, row := range rows { + out = append(out, adminDiagnosticDeviceView{ + ClientID: row.ClientID, + UserID: uuidToString(row.UserID), + Username: row.Username, + AppVersion: row.AppVersion, + OsVersion: row.OsVersion, + LastSeen: formatTimestamp(row.LastSeen), + EventCount: row.EventCount, + }) + } + writeJSON(w, http.StatusOK, out) +} + +// adminDebugModeReq is the body of PUT /api/admin/users/{id}/debug-mode. +type adminDebugModeReq struct { + Enabled bool `json:"enabled"` +} + +// handleAdminDebugModeToggle implements PUT +// /api/admin/users/{id}/debug-mode — flip an account's diagnostics +// opt-in remotely while a bug is live. Echoes the updated user. +func (h *handlers) handleAdminDebugModeToggle(w http.ResponseWriter, r *http.Request) { + targetID, ok := requireURLUUID(w, r, "id") + if !ok { + return + } + var req adminDebugModeReq + if !decodeBody(w, r, &req) { + return + } + updated, err := dbq.New(h.pool).SetDebugMode(r.Context(), dbq.SetDebugModeParams{ + ID: targetID, + DebugModeEnabled: req.Enabled, + }) + if err != nil { + writeErrWithLog(w, h.logger, "admin: set debug mode failed", apierror.Internal(err)) + return + } + h.logger.Info("admin: debug mode toggled", + "target_user", uuidToString(targetID), "enabled", req.Enabled) + writeJSON(w, http.StatusOK, adminUserView{ + ID: uuidToString(updated.ID), + Username: updated.Username, + DisplayName: updated.DisplayName, + IsAdmin: updated.IsAdmin, + AutoApproveRequests: updated.AutoApproveRequests, + DebugModeEnabled: updated.DebugModeEnabled, + CreatedAt: updated.CreatedAt.Time.UTC().Format(time.RFC3339), + }) +} + +// parseDiagPaging reads limit/offset with diagnostics-specific caps. +// Blank → defaults; out-of-range clamps; non-numeric → error. +func parseDiagPaging(rawLimit, rawOffset string) (limit, offset int, err error) { + limit = defaultDiagnosticPageSize + if s := strings.TrimSpace(rawLimit); s != "" { + n, perr := strconv.Atoi(s) + if perr != nil { + return 0, 0, perr + } + if n < 1 { + n = 1 + } + if n > maxDiagnosticPageSize { + n = maxDiagnosticPageSize + } + limit = n + } + if s := strings.TrimSpace(rawOffset); s != "" { + n, perr := strconv.Atoi(s) + if perr != nil { + return 0, 0, perr + } + if n > 0 { + offset = n + } + } + return limit, offset, nil +} + +// optionalQuery returns nil for an absent/blank query value so the sqlc +// NULL-or-equals filter treats it as "no filter." +func optionalQuery(raw string) *string { + raw = strings.TrimSpace(raw) + if raw == "" { + return nil + } + return &raw +} + +// parseDiagTime accepts RFC3339 or epoch-millis. Empty = absent (NULL +// filter, ok=true). Returns ok=false only on a non-empty unparseable value. +func parseDiagTime(raw string) (pgtype.Timestamptz, bool) { + raw = strings.TrimSpace(raw) + if raw == "" { + return pgtype.Timestamptz{}, true + } + if t, err := time.Parse(time.RFC3339, raw); err == nil { + return pgtype.Timestamptz{Time: t.UTC(), Valid: true}, true + } + if ms, err := strconv.ParseInt(raw, 10, 64); err == nil { + return pgtype.Timestamptz{Time: time.UnixMilli(ms).UTC(), Valid: true}, true + } + return pgtype.Timestamptz{}, false +} diff --git a/internal/api/me_profile.go b/internal/api/me_profile.go index 349fa165..ce5f64cf 100644 --- a/internal/api/me_profile.go +++ b/internal/api/me_profile.go @@ -35,6 +35,10 @@ type meProfileResp struct { DisplayName *string `json:"display_name"` Email *string `json:"email"` IsAdmin bool `json:"is_admin"` + // DebugModeEnabled is the account's diagnostics opt-in. The client + // reads it here to decide whether to run the on-device diagnostics + // reporter (M9). Admin-set; the client also has a local OFF switch. + DebugModeEnabled bool `json:"debug_mode_enabled"` } func (h *handlers) handleUpdateMyProfile(w http.ResponseWriter, r *http.Request) { @@ -120,10 +124,11 @@ func (h *handlers) handleUpdateMyProfile(w http.ResponseWriter, r *http.Request) func profileViewFromUser(u dbq.User) meProfileResp { return meProfileResp{ - ID: uuidToString(u.ID), - Username: u.Username, - DisplayName: u.DisplayName, - Email: u.Email, - IsAdmin: u.IsAdmin, + ID: uuidToString(u.ID), + Username: u.Username, + DisplayName: u.DisplayName, + Email: u.Email, + IsAdmin: u.IsAdmin, + DebugModeEnabled: u.DebugModeEnabled, } } diff --git a/internal/db/dbq/diagnostics.sql.go b/internal/db/dbq/diagnostics.sql.go new file mode 100644 index 00000000..f8814239 --- /dev/null +++ b/internal/db/dbq/diagnostics.sql.go @@ -0,0 +1,221 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: diagnostics.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const gcPruneDiagnostics = `-- name: GcPruneDiagnostics :execrows +DELETE FROM diagnostic_events WHERE received_at < now() - INTERVAL '30 days' +` + +// Retention sweep for the gc worker. Deletes diagnostic_events older +// than 30 days by server clock (received_at) so a skewed client clock +// can't keep rows alive. Matches the other gc lifecycle sweeps' +// hardcoded-threshold style; diagnostics are debug telemetry, not user +// data, and 30 days is ample to investigate a live incident after the +// fact. Returns the affected row count for the sweep log line. +func (q *Queries) GcPruneDiagnostics(ctx context.Context) (int64, error) { + result, err := q.db.Exec(ctx, gcPruneDiagnostics) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const insertDiagnosticEvent = `-- name: InsertDiagnosticEvent :one +INSERT INTO diagnostic_events ( + user_id, client_id, app_version, os_version, kind, payload, occurred_at +) +VALUES ($1, $2, $3, $4, $5, $6, $7) +RETURNING id, received_at +` + +type InsertDiagnosticEventParams struct { + UserID pgtype.UUID + ClientID string + AppVersion *string + OsVersion *string + Kind string + Payload []byte + OccurredAt pgtype.Timestamptz +} + +type InsertDiagnosticEventRow struct { + ID pgtype.UUID + ReceivedAt pgtype.Timestamptz +} + +// Records one client-reported diagnostic event. The writer validates the +// `kind` category against its whitelist before this runs; the CHECK +// constraint is the belt-and-braces guard. payload carries the finer +// event discriminator + all event-specific fields. Returns received_at +// so the caller can confirm ingest. +func (q *Queries) InsertDiagnosticEvent(ctx context.Context, arg InsertDiagnosticEventParams) (InsertDiagnosticEventRow, error) { + row := q.db.QueryRow(ctx, insertDiagnosticEvent, + arg.UserID, + arg.ClientID, + arg.AppVersion, + arg.OsVersion, + arg.Kind, + arg.Payload, + arg.OccurredAt, + ) + var i InsertDiagnosticEventRow + err := row.Scan(&i.ID, &i.ReceivedAt) + return i, err +} + +const listAdminDiagnostics = `-- name: ListAdminDiagnostics :many +SELECT + de.id AS id, + de.user_id AS user_id, + u.username AS username, + de.client_id AS client_id, + de.app_version AS app_version, + de.os_version AS os_version, + de.kind AS kind, + de.payload AS payload, + de.occurred_at AS occurred_at, + de.received_at AS received_at +FROM diagnostic_events de +JOIN users u ON u.id = de.user_id +WHERE ($1::uuid IS NULL OR de.user_id = $1::uuid) + AND ($2::text IS NULL OR de.client_id = $2::text) + AND ($3::text IS NULL OR de.kind = $3::text) + AND ($4::timestamptz IS NULL + OR de.occurred_at >= $4::timestamptz) + AND ($5::timestamptz IS NULL + OR de.occurred_at <= $5::timestamptz) +ORDER BY de.occurred_at DESC +LIMIT $7::int OFFSET $6::int +` + +type ListAdminDiagnosticsParams struct { + UserID pgtype.UUID + ClientID *string + Kind *string + FromTs pgtype.Timestamptz + ToTs pgtype.Timestamptz + Off int32 + Lim int32 +} + +type ListAdminDiagnosticsRow struct { + ID pgtype.UUID + UserID pgtype.UUID + Username string + ClientID string + AppVersion *string + OsVersion *string + Kind string + Payload []byte + OccurredAt pgtype.Timestamptz + ReceivedAt pgtype.Timestamptz +} + +// Admin timeline query. All filters are optional (NULL = no filter): +// account (user_id), device (client_id), category (kind), and the +// occurred_at window (from_ts/to_ts). Newest-first; the SPA reverses for +// a chronological timeline / export. Joined with users for the username +// so the view renders without a second round-trip. +func (q *Queries) ListAdminDiagnostics(ctx context.Context, arg ListAdminDiagnosticsParams) ([]ListAdminDiagnosticsRow, error) { + rows, err := q.db.Query(ctx, listAdminDiagnostics, + arg.UserID, + arg.ClientID, + arg.Kind, + arg.FromTs, + arg.ToTs, + arg.Off, + arg.Lim, + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAdminDiagnosticsRow + for rows.Next() { + var i ListAdminDiagnosticsRow + if err := rows.Scan( + &i.ID, + &i.UserID, + &i.Username, + &i.ClientID, + &i.AppVersion, + &i.OsVersion, + &i.Kind, + &i.Payload, + &i.OccurredAt, + &i.ReceivedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listDiagnosticDevices = `-- name: ListDiagnosticDevices :many +SELECT + de.client_id AS client_id, + de.user_id AS user_id, + u.username AS username, + max(de.app_version)::text AS app_version, + max(de.os_version)::text AS os_version, + max(de.occurred_at)::timestamptz AS last_seen, + count(*) AS event_count +FROM diagnostic_events de +JOIN users u ON u.id = de.user_id +WHERE ($1::uuid IS NULL OR de.user_id = $1::uuid) +GROUP BY de.client_id, de.user_id, u.username +ORDER BY last_seen DESC +` + +type ListDiagnosticDevicesRow struct { + ClientID string + UserID pgtype.UUID + Username string + AppVersion string + OsVersion string + LastSeen pgtype.Timestamptz + EventCount int64 +} + +// Distinct devices that have reported, for the admin device filter + +// "who is currently reporting" overview. Optionally scoped to one account. +func (q *Queries) ListDiagnosticDevices(ctx context.Context, userID pgtype.UUID) ([]ListDiagnosticDevicesRow, error) { + rows, err := q.db.Query(ctx, listDiagnosticDevices, userID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListDiagnosticDevicesRow + for rows.Next() { + var i ListDiagnosticDevicesRow + if err := rows.Scan( + &i.ClientID, + &i.UserID, + &i.Username, + &i.AppVersion, + &i.OsVersion, + &i.LastSeen, + &i.EventCount, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index a30f8d86..bdf64e72 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -265,6 +265,18 @@ type CoverArtSourcesMetum struct { LastRegisteredProvidersHash string } +type DiagnosticEvent struct { + ID pgtype.UUID + UserID pgtype.UUID + ClientID string + AppVersion *string + OsVersion *string + Kind string + Payload []byte + OccurredAt pgtype.Timestamptz + ReceivedAt pgtype.Timestamptz +} + type GeneralLike struct { UserID pgtype.UUID TrackID pgtype.UUID @@ -552,6 +564,7 @@ type User struct { Email *string Timezone string TimezoneUpdatedAt pgtype.Timestamptz + DebugModeEnabled bool } type UserInvite struct { diff --git a/internal/db/dbq/users.sql.go b/internal/db/dbq/users.sql.go index 3f1b9722..fe5fea6f 100644 --- a/internal/db/dbq/users.sql.go +++ b/internal/db/dbq/users.sql.go @@ -54,7 +54,7 @@ func (q *Queries) CountUsers(ctx context.Context) (int64, error) { const createUser = `-- name: CreateUser :one INSERT INTO users (username, password_hash, api_token, is_admin, display_name) VALUES ($1, $2, $3, $4, $5) -RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at +RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled ` type CreateUserParams struct { @@ -89,6 +89,7 @@ func (q *Queries) CreateUser(ctx context.Context, arg CreateUserParams) (User, e &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } @@ -96,7 +97,7 @@ func (q *Queries) CreateUser(ctx context.Context, arg CreateUserParams) (User, e const createUserAdmin = `-- name: CreateUserAdmin :one INSERT INTO users (username, password_hash, api_token, is_admin, display_name) VALUES ($1, $2, $3, $4, $5) -RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at +RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled ` type CreateUserAdminParams struct { @@ -134,6 +135,7 @@ func (q *Queries) CreateUserAdmin(ctx context.Context, arg CreateUserAdminParams &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } @@ -145,7 +147,7 @@ VALUES ( (SELECT NOT EXISTS (SELECT 1 FROM users)), $4 ) -RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at +RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled ` type CreateUserFirstAdminRaceParams struct { @@ -191,6 +193,7 @@ func (q *Queries) CreateUserFirstAdminRace(ctx context.Context, arg CreateUserFi &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } @@ -237,7 +240,7 @@ func (q *Queries) GetListenBrainzConfig(ctx context.Context, id pgtype.UUID) (Ge } const getUserByAPIToken = `-- name: GetUserByAPIToken :one -SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at FROM users WHERE api_token = $1 +SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled FROM users WHERE api_token = $1 ` func (q *Queries) GetUserByAPIToken(ctx context.Context, apiToken string) (User, error) { @@ -258,12 +261,13 @@ func (q *Queries) GetUserByAPIToken(ctx context.Context, apiToken string) (User, &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } const getUserByEmail = `-- name: GetUserByEmail :one -SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at FROM users WHERE lower(email) = lower($1) +SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled FROM users WHERE lower(email) = lower($1) ` // Used by forgot-password lookup. Lowercase comparison both sides @@ -287,12 +291,13 @@ func (q *Queries) GetUserByEmail(ctx context.Context, lower string) (User, error &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } const getUserByID = `-- name: GetUserByID :one -SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at FROM users WHERE id = $1 +SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled FROM users WHERE id = $1 ` func (q *Queries) GetUserByID(ctx context.Context, id pgtype.UUID) (User, error) { @@ -313,12 +318,13 @@ func (q *Queries) GetUserByID(ctx context.Context, id pgtype.UUID) (User, error) &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } const getUserByUsername = `-- name: GetUserByUsername :one -SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at FROM users WHERE username = $1 +SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled FROM users WHERE username = $1 ` func (q *Queries) GetUserByUsername(ctx context.Context, username string) (User, error) { @@ -339,6 +345,7 @@ func (q *Queries) GetUserByUsername(ctx context.Context, username string) (User, &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } @@ -382,7 +389,8 @@ func (q *Queries) ListActiveUsersWithTimezones(ctx context.Context) ([]ListActiv } const listUsers = `-- name: ListUsers :many -SELECT id, username, display_name, is_admin, auto_approve_requests, created_at +SELECT id, username, display_name, is_admin, auto_approve_requests, + debug_mode_enabled, created_at FROM users ORDER BY created_at DESC ` @@ -393,6 +401,7 @@ type ListUsersRow struct { DisplayName *string IsAdmin bool AutoApproveRequests bool + DebugModeEnabled bool CreatedAt pgtype.Timestamptz } @@ -412,6 +421,7 @@ func (q *Queries) ListUsers(ctx context.Context) ([]ListUsersRow, error) { &i.DisplayName, &i.IsAdmin, &i.AutoApproveRequests, + &i.DebugModeEnabled, &i.CreatedAt, ); err != nil { return nil, err @@ -426,7 +436,7 @@ func (q *Queries) ListUsers(ctx context.Context) ([]ListUsersRow, error) { const regenerateApiToken = `-- name: RegenerateApiToken :one UPDATE users SET api_token = $2 WHERE id = $1 -RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at +RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled ` type RegenerateApiTokenParams struct { @@ -454,6 +464,7 @@ func (q *Queries) RegenerateApiToken(ctx context.Context, arg RegenerateApiToken &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } @@ -478,6 +489,44 @@ func (q *Queries) ResetUserPassword(ctx context.Context, arg ResetUserPasswordPa return err } +const setDebugMode = `-- name: SetDebugMode :one +UPDATE users + SET debug_mode_enabled = $2 + WHERE id = $1 +RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled +` + +type SetDebugModeParams struct { + ID pgtype.UUID + DebugModeEnabled bool +} + +// Toggle the per-account diagnostics/debug-reporting opt-in. Admin-driven +// from /admin (flip remotely while a bug is live) or self-driven OFF from +// the client. Returns the updated row so the handler can echo it. +func (q *Queries) SetDebugMode(ctx context.Context, arg SetDebugModeParams) (User, error) { + row := q.db.QueryRow(ctx, setDebugMode, arg.ID, arg.DebugModeEnabled) + var i User + err := row.Scan( + &i.ID, + &i.Username, + &i.PasswordHash, + &i.ApiToken, + &i.IsAdmin, + &i.CreatedAt, + &i.SubsonicPassword, + &i.ListenbrainzToken, + &i.ListenbrainzEnabled, + &i.DisplayName, + &i.AutoApproveRequests, + &i.Email, + &i.Timezone, + &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, + ) + return i, err +} + const setListenBrainzEnabled = `-- name: SetListenBrainzEnabled :exec UPDATE users SET listenbrainz_enabled = $2 @@ -531,7 +580,7 @@ const updateUserAdmin = `-- name: UpdateUserAdmin :one UPDATE users SET is_admin = $2 WHERE id = $1 -RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at +RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled ` type UpdateUserAdminParams struct { @@ -559,6 +608,7 @@ func (q *Queries) UpdateUserAdmin(ctx context.Context, arg UpdateUserAdminParams &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } @@ -567,7 +617,7 @@ const updateUserAutoApprove = `-- name: UpdateUserAutoApprove :one UPDATE users SET auto_approve_requests = $2 WHERE id = $1 -RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at +RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled ` type UpdateUserAutoApproveParams struct { @@ -594,6 +644,7 @@ func (q *Queries) UpdateUserAutoApprove(ctx context.Context, arg UpdateUserAutoA &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } @@ -603,7 +654,7 @@ UPDATE users SET display_name = $2, email = $3 WHERE id = $1 -RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at +RETURNING id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled ` type UpdateUserProfileParams struct { @@ -632,6 +683,7 @@ func (q *Queries) UpdateUserProfile(ctx context.Context, arg UpdateUserProfilePa &i.Email, &i.Timezone, &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, ) return i, err } diff --git a/internal/db/migrations/0036_diagnostics.down.sql b/internal/db/migrations/0036_diagnostics.down.sql new file mode 100644 index 00000000..b53c33ba --- /dev/null +++ b/internal/db/migrations/0036_diagnostics.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE users DROP COLUMN IF EXISTS debug_mode_enabled; +DROP TABLE IF EXISTS diagnostic_events; diff --git a/internal/db/migrations/0036_diagnostics.up.sql b/internal/db/migrations/0036_diagnostics.up.sql new file mode 100644 index 00000000..37876893 --- /dev/null +++ b/internal/db/migrations/0036_diagnostics.up.sql @@ -0,0 +1,54 @@ +-- Device diagnostics / debug-reporting subsystem (M9). +-- +-- When an account has users.debug_mode_enabled = true, that account's +-- client(s) stream a timeseries of device state here: connectivity + +-- server-health transitions (roaming / poor-data recovery), UPnP/Sonos +-- sync events (locked-phone desync), and power/lifecycle state (Doze / +-- battery-optimization, the prime suspect for the desync). The admin +-- /admin/diagnostics view filters by account + device + time window and +-- exports the slice as JSON for offline analysis. +-- +-- `kind` is a COARSE category gated by a CHECK whitelist; the finer +-- event discriminator + all event-specific fields live in the `payload` +-- jsonb. This keeps the enum small and stable so new event variants +-- don't require a migration (per the enum-CHECK-whitelist rule, only a +-- new *category* would). If you add a category here, mirror it in the +-- handler whitelist in internal/api/diagnostics.go. +-- +-- Two timestamps on purpose: occurred_at is the client device clock +-- (when the event happened on the phone — may be skewed/wrong, e.g. in +-- a dead zone), received_at is the server clock at ingest. Retention +-- pruning is keyed on received_at so a bad client clock can't keep rows +-- alive forever. + +CREATE TABLE diagnostic_events ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + client_id text NOT NULL, + app_version text, + os_version text, + kind text NOT NULL, + payload jsonb NOT NULL DEFAULT '{}'::jsonb, + occurred_at timestamptz NOT NULL, + received_at timestamptz NOT NULL DEFAULT now(), + CONSTRAINT diagnostic_events_kind_check + CHECK (kind IN ( + 'connectivity', 'upnp_sync', 'power', 'lifecycle', + 'heartbeat', 'http' + )) +); + +-- Admin reads filter by account then device, ordered newest-first. +CREATE INDEX idx_diagnostic_events_user + ON diagnostic_events (user_id, occurred_at DESC); +CREATE INDEX idx_diagnostic_events_client + ON diagnostic_events (client_id, occurred_at DESC); +-- Retention pruner deletes by server-clock age. +CREATE INDEX idx_diagnostic_events_received + ON diagnostic_events (received_at); + +-- Per-account opt-in for diagnostics reporting. Admin-set primarily +-- (flip remotely while a bug is live); the client also exposes a local +-- OFF switch. Default false: no account reports until explicitly enabled. +ALTER TABLE users + ADD COLUMN IF NOT EXISTS debug_mode_enabled boolean NOT NULL DEFAULT false; diff --git a/internal/db/queries/diagnostics.sql b/internal/db/queries/diagnostics.sql new file mode 100644 index 00000000..2aa36f74 --- /dev/null +++ b/internal/db/queries/diagnostics.sql @@ -0,0 +1,66 @@ +-- name: InsertDiagnosticEvent :one +-- Records one client-reported diagnostic event. The writer validates the +-- `kind` category against its whitelist before this runs; the CHECK +-- constraint is the belt-and-braces guard. payload carries the finer +-- event discriminator + all event-specific fields. Returns received_at +-- so the caller can confirm ingest. +INSERT INTO diagnostic_events ( + user_id, client_id, app_version, os_version, kind, payload, occurred_at +) +VALUES ($1, $2, $3, $4, $5, $6, $7) +RETURNING id, received_at; + +-- name: ListAdminDiagnostics :many +-- Admin timeline query. All filters are optional (NULL = no filter): +-- account (user_id), device (client_id), category (kind), and the +-- occurred_at window (from_ts/to_ts). Newest-first; the SPA reverses for +-- a chronological timeline / export. Joined with users for the username +-- so the view renders without a second round-trip. +SELECT + de.id AS id, + de.user_id AS user_id, + u.username AS username, + de.client_id AS client_id, + de.app_version AS app_version, + de.os_version AS os_version, + de.kind AS kind, + de.payload AS payload, + de.occurred_at AS occurred_at, + de.received_at AS received_at +FROM diagnostic_events de +JOIN users u ON u.id = de.user_id +WHERE (sqlc.narg(user_id)::uuid IS NULL OR de.user_id = sqlc.narg(user_id)::uuid) + AND (sqlc.narg(client_id)::text IS NULL OR de.client_id = sqlc.narg(client_id)::text) + AND (sqlc.narg(kind)::text IS NULL OR de.kind = sqlc.narg(kind)::text) + AND (sqlc.narg(from_ts)::timestamptz IS NULL + OR de.occurred_at >= sqlc.narg(from_ts)::timestamptz) + AND (sqlc.narg(to_ts)::timestamptz IS NULL + OR de.occurred_at <= sqlc.narg(to_ts)::timestamptz) +ORDER BY de.occurred_at DESC +LIMIT sqlc.arg(lim)::int OFFSET sqlc.arg(off)::int; + +-- name: ListDiagnosticDevices :many +-- Distinct devices that have reported, for the admin device filter + +-- "who is currently reporting" overview. Optionally scoped to one account. +SELECT + de.client_id AS client_id, + de.user_id AS user_id, + u.username AS username, + max(de.app_version)::text AS app_version, + max(de.os_version)::text AS os_version, + max(de.occurred_at)::timestamptz AS last_seen, + count(*) AS event_count +FROM diagnostic_events de +JOIN users u ON u.id = de.user_id +WHERE (sqlc.narg(user_id)::uuid IS NULL OR de.user_id = sqlc.narg(user_id)::uuid) +GROUP BY de.client_id, de.user_id, u.username +ORDER BY last_seen DESC; + +-- name: GcPruneDiagnostics :execrows +-- Retention sweep for the gc worker. Deletes diagnostic_events older +-- than 30 days by server clock (received_at) so a skewed client clock +-- can't keep rows alive. Matches the other gc lifecycle sweeps' +-- hardcoded-threshold style; diagnostics are debug telemetry, not user +-- data, and 30 days is ample to investigate a live incident after the +-- fact. Returns the affected row count for the sweep log line. +DELETE FROM diagnostic_events WHERE received_at < now() - INTERVAL '30 days'; diff --git a/internal/db/queries/users.sql b/internal/db/queries/users.sql index 800e1b7a..971a60b3 100644 --- a/internal/db/queries/users.sql +++ b/internal/db/queries/users.sql @@ -55,7 +55,8 @@ WHERE id = $1; -- name: ListUsers :many -- Admin user-management list. Sort newest-first. -SELECT id, username, display_name, is_admin, auto_approve_requests, created_at +SELECT id, username, display_name, is_admin, auto_approve_requests, + debug_mode_enabled, created_at FROM users ORDER BY created_at DESC; @@ -115,6 +116,15 @@ UPDATE users WHERE id = $1 RETURNING *; +-- name: SetDebugMode :one +-- Toggle the per-account diagnostics/debug-reporting opt-in. Admin-driven +-- from /admin (flip remotely while a bug is live) or self-driven OFF from +-- the client. Returns the updated row so the handler can echo it. +UPDATE users + SET debug_mode_enabled = $2 + WHERE id = $1 +RETURNING *; + -- name: ChangeUserPassword :exec -- Self-service password change. Caller (HTTP handler) verifies the -- current password before calling this. Distinct from diff --git a/internal/gc/worker.go b/internal/gc/worker.go index 22b42269..b133e6c4 100644 --- a/internal/gc/worker.go +++ b/internal/gc/worker.go @@ -16,6 +16,7 @@ // - GcExpireScrobbleQueueFailedRows (#567) // - GcResetStuckSystemPlaylistRuns (#574) // - GcDeleteExpiredPasswordResets (#575) +// - GcPruneDiagnostics (M9 — diagnostics 30d retention) package gc import ( @@ -82,6 +83,7 @@ func (w *Worker) tickOnce(ctx context.Context) { w.runSweep(ctx, "expire_scrobble_failed", q.GcExpireScrobbleQueueFailedRows) w.runSweep(ctx, "reset_stuck_system_runs", q.GcResetStuckSystemPlaylistRuns) w.runSweep(ctx, "delete_expired_password_resets", q.GcDeleteExpiredPasswordResets) + w.runSweep(ctx, "prune_diagnostics", q.GcPruneDiagnostics) } // runSweep is a small adapter so each sweep call site is a one-liner From 4d42e298dd16e55e8a57f39df117f0bf6c901432 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 29 Jun 2026 18:56:36 -0400 Subject: [PATCH 2/5] feat(android+web/diagnostics): on-device debug reporter + admin timeline (M9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android: a gated DiagnosticsReporter taps connectivity, server-health, UPnP drops/player-state/route, power (Doze/battery-opt/screen), and app fg/bg, plus a heartbeat snapshotting Sonos-vs-local position — the locked-phone desync signal. Events buffer in a Room ring buffer (deliberately NOT the MutationQueue: high-volume best-effort telemetry that must survive the dead zone being debugged) and DiagnosticsUploader drains them on a tick / health-recovery / sign-in. Gating: the account flag (users.debug_mode_enabled) reaches the device via a new /api/me refresh in AuthController; a per-device local OFF switch lives in Settings. Reporter runs only when enabled && !optOut; disabling drops the unsent buffer. Web admin: /admin/diagnostics — pick account+device+kind+time-window, see a chronological timeline, flip an account's debug mode remotely, and Copy-JSON / Download-NDJSON the slice for analysis. Room schema 6→7 (new diagnostic_events table + auth_session.diagnosticsOptOut; pre-v1 destructive fallback). Refs Scribe M9 (#119), tasks #1174 #1175 #1176 #1177. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- .../minstrel/MinstrelApplication.kt | 7 + .../minstrel/api/endpoints/DiagnosticsApi.kt | 36 +++ .../minstrel/auth/AuthController.kt | 29 ++ .../fabledsword/minstrel/auth/AuthStore.kt | 18 ++ .../minstrel/cache/db/AppDatabase.kt | 8 +- .../minstrel/cache/db/DatabaseModule.kt | 6 + .../minstrel/cache/db/dao/AuthSessionDao.kt | 4 + .../cache/db/dao/DiagnosticEventDao.kt | 35 ++ .../cache/db/entities/AuthSessionEntity.kt | 7 + .../db/entities/DiagnosticEventEntity.kt | 28 ++ .../diagnostics/DiagnosticsReporter.kt | 284 ++++++++++++++++ .../diagnostics/DiagnosticsUploader.kt | 115 +++++++ .../fabledsword/minstrel/models/UserRef.kt | 7 + .../minstrel/models/wire/MyProfileWire.kt | 1 + .../minstrel/settings/ui/SettingsScreen.kt | 49 +++ .../minstrel/settings/ui/SettingsViewModel.kt | 15 +- web/src/lib/api/admin.ts | 86 +++++ web/src/lib/api/queries.ts | 4 + web/src/lib/components/AdminTabs.svelte | 1 + web/src/routes/admin/diagnostics/+page.svelte | 302 ++++++++++++++++++ 20 files changed, 1039 insertions(+), 3 deletions(-) create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/DiagnosticsApi.kt create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/DiagnosticEventDao.kt create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/cache/db/entities/DiagnosticEventEntity.kt create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsUploader.kt create mode 100644 web/src/routes/admin/diagnostics/+page.svelte diff --git a/android/app/src/main/java/com/fabledsword/minstrel/MinstrelApplication.kt b/android/app/src/main/java/com/fabledsword/minstrel/MinstrelApplication.kt index 18db4513..65a34df6 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/MinstrelApplication.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/MinstrelApplication.kt @@ -10,6 +10,8 @@ import com.fabledsword.minstrel.cache.CacheIndexer import com.fabledsword.minstrel.cache.mutations.MutationReplayer import com.fabledsword.minstrel.cache.sync.SyncController import com.fabledsword.minstrel.di.ApplicationScope +import com.fabledsword.minstrel.diagnostics.DiagnosticsReporter +import com.fabledsword.minstrel.diagnostics.DiagnosticsUploader import com.fabledsword.minstrel.events.EventsStream import com.fabledsword.minstrel.events.LiveEventsDispatcher import com.fabledsword.minstrel.metadata.FreshnessSweeper @@ -155,6 +157,11 @@ class MinstrelApplication : */ @Suppress("unused") @Inject lateinit var liveEventsDispatcher: LiveEventsDispatcher + // Device diagnostics (M9). The reporter gates itself on the account's + // debug flag; the uploader drains its buffer on a tick / recovery. + @Suppress("unused") @Inject lateinit var diagnosticsReporter: DiagnosticsReporter + @Suppress("unused") @Inject lateinit var diagnosticsUploader: DiagnosticsUploader + @Inject @ApplicationScope lateinit var appScope: CoroutineScope override fun onCreate() { diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/DiagnosticsApi.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/DiagnosticsApi.kt new file mode 100644 index 00000000..b6edd751 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/DiagnosticsApi.kt @@ -0,0 +1,36 @@ +package com.fabledsword.minstrel.api.endpoints + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonElement +import retrofit2.http.Body +import retrofit2.http.POST + +/** + * Retrofit interface for device diagnostics ingest (M9). One call + * uploads a batch of buffered events. The server stores them only when + * the account's debug_mode_enabled flag is on (it returns 204 otherwise, + * which the uploader treats as success so it can drop the batch). + */ +interface DiagnosticsApi { + @POST("api/diagnostics") + suspend fun report(@Body body: DiagnosticsReportRequest) +} + +@Serializable +data class DiagnosticsReportRequest( + @SerialName("client_id") val clientId: String, + @SerialName("app_version") val appVersion: String? = null, + @SerialName("os_version") val osVersion: String? = null, + val events: List, +) + +@Serializable +data class DiagnosticEventWire( + val kind: String, + // Device-clock epoch milliseconds. The server stamps its own + // received_at; both are stored so a skewed device clock is visible. + @SerialName("occurred_at") val occurredAt: Long, + // Opaque structured payload carrying the event sub-type + fields. + val payload: JsonElement, +) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthController.kt b/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthController.kt index 419a815c..54cb281b 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthController.kt @@ -1,6 +1,7 @@ package com.fabledsword.minstrel.auth import com.fabledsword.minstrel.api.endpoints.AuthApi +import com.fabledsword.minstrel.api.endpoints.MeApi import com.fabledsword.minstrel.di.ApplicationScope import com.fabledsword.minstrel.models.UserRef import com.fabledsword.minstrel.models.wire.LoginRequestBody @@ -35,6 +36,7 @@ class AuthController @Inject constructor( retrofit: Retrofit, ) { private val api: AuthApi = retrofit.create() + private val meApi: MeApi = retrofit.create() private val currentUserState = MutableStateFlow(null) val currentUser: StateFlow = currentUserState.asStateFlow() @@ -55,6 +57,10 @@ class AuthController @Inject constructor( currentUserState.value = raw?.let { decodeUser(it) } } } + // Refresh from /api/me on startup so a remotely-changed account + // flag (e.g. admin enabling debug mode, M9) reaches the device + // without a re-login. Best-effort; failures keep the cached value. + scope.launch { refreshProfile() } } /** @@ -72,9 +78,32 @@ class AuthController @Inject constructor( ) currentUserState.value = user authStore.setUserJson(json.encodeToString(UserRef.serializer(), user)) + // Pull the fuller /me shape (carries debug_mode_enabled) right + // after login so the diagnostics gate is correct without waiting + // for the next startup refresh. + scope.launch { refreshProfile() } return user } + /** + * Re-fetch the caller's profile from /api/me and update currentUser + * + persisted userJson. Carries account-level flags (debug mode) + * that aren't in the login response. Best-effort: a network failure + * leaves the cached identity untouched. No-op when signed out. + */ + suspend fun refreshProfile() { + if (!isSignedIn) return + val p = runCatching { meApi.getProfile() }.getOrNull() ?: return + val user = UserRef( + id = p.id, + username = p.username, + isAdmin = p.isAdmin, + debugModeEnabled = p.debugModeEnabled, + ) + currentUserState.value = user + authStore.setUserJson(json.encodeToString(UserRef.serializer(), user)) + } + /** * Clears the local session immediately and best-effort hits * `/api/auth/logout` so the server can drop its session row. diff --git a/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthStore.kt b/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthStore.kt index c4f493f8..f4aeabc1 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthStore.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthStore.kt @@ -59,6 +59,9 @@ class AuthStore @Inject constructor( private val cacheSettingsState = MutableStateFlow(CacheSettings.DEFAULT) val cacheSettings: StateFlow = cacheSettingsState.asStateFlow() + private val diagnosticsOptOutState = MutableStateFlow(false) + val diagnosticsOptOut: StateFlow = diagnosticsOptOutState.asStateFlow() + private val json = Json { ignoreUnknownKeys = true } init { @@ -70,6 +73,7 @@ class AuthStore @Inject constructor( themeModeState.value = row?.themeMode clientIdState.value = row?.clientId cacheSettingsState.value = decodeCacheSettings(row?.cacheSettingsJson) + diagnosticsOptOutState.value = row?.diagnosticsOptOut ?: false } } } @@ -112,6 +116,11 @@ class AuthStore @Inject constructor( scope.launch { persistCacheSettings(encoded) } } + fun setDiagnosticsOptOut(value: Boolean) { + diagnosticsOptOutState.value = value + scope.launch { persistDiagnosticsOptOut(value) } + } + private suspend fun persistCookie(value: String?) { if (dao.get() == null) { dao.upsert(currentEntity().copy(sessionCookie = value)) @@ -160,6 +169,14 @@ class AuthStore @Inject constructor( } } + private suspend fun persistDiagnosticsOptOut(value: Boolean) { + if (dao.get() == null) { + dao.upsert(currentEntity().copy(diagnosticsOptOut = value)) + } else { + dao.setDiagnosticsOptOut(value) + } + } + private fun currentEntity(): AuthSessionEntity = AuthSessionEntity( id = ROW_ID, sessionCookie = sessionCookieState.value, @@ -171,6 +188,7 @@ class AuthStore @Inject constructor( CacheSettings.serializer(), cacheSettingsState.value, ), + diagnosticsOptOut = diagnosticsOptOutState.value, ) companion object { diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/AppDatabase.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/AppDatabase.kt index 8e2d4d83..5e5c9306 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/AppDatabase.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/AppDatabase.kt @@ -16,6 +16,7 @@ import com.fabledsword.minstrel.cache.db.dao.CachedResumeStateDao import com.fabledsword.minstrel.cache.db.dao.CachedPlaylistTrackDao import com.fabledsword.minstrel.cache.db.dao.CachedQuarantineDao import com.fabledsword.minstrel.cache.db.dao.CachedTrackDao +import com.fabledsword.minstrel.cache.db.dao.DiagnosticEventDao import com.fabledsword.minstrel.cache.db.dao.SyncMetadataDao import com.fabledsword.minstrel.cache.db.entities.AudioCacheIndexEntity import com.fabledsword.minstrel.cache.db.entities.AuthSessionEntity @@ -30,6 +31,7 @@ import com.fabledsword.minstrel.cache.db.entities.CachedResumeStateEntity import com.fabledsword.minstrel.cache.db.entities.CachedPlaylistTrackEntity import com.fabledsword.minstrel.cache.db.entities.CachedQuarantineEntity import com.fabledsword.minstrel.cache.db.entities.CachedTrackEntity +import com.fabledsword.minstrel.cache.db.entities.DiagnosticEventEntity import com.fabledsword.minstrel.cache.db.entities.SyncMetadataEntity /** @@ -61,8 +63,11 @@ import com.fabledsword.minstrel.cache.db.entities.SyncMetadataEntity CachedHomeIndexEntity::class, CachedHistorySnapshotEntity::class, AuthSessionEntity::class, + DiagnosticEventEntity::class, ], - version = 6, + // v7: + diagnostic_events table (M9) and the diagnosticsOptOut column + // on auth_session. Pre-v1 destructive fallback rebuilds on mismatch. + version = 7, exportSchema = true, ) @TypeConverters(MinstrelTypeConverters::class) @@ -81,4 +86,5 @@ abstract class AppDatabase : RoomDatabase() { abstract fun cachedHomeIndexDao(): CachedHomeIndexDao abstract fun cachedHistorySnapshotDao(): CachedHistorySnapshotDao abstract fun authSessionDao(): AuthSessionDao + abstract fun diagnosticEventDao(): DiagnosticEventDao } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/DatabaseModule.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/DatabaseModule.kt index ece05791..c5c58df4 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/DatabaseModule.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/DatabaseModule.kt @@ -10,6 +10,7 @@ import com.fabledsword.minstrel.cache.db.dao.CachedHistorySnapshotDao import com.fabledsword.minstrel.cache.db.dao.CachedHomeIndexDao import com.fabledsword.minstrel.cache.db.dao.CachedLikeDao import com.fabledsword.minstrel.cache.db.dao.CachedMutationDao +import com.fabledsword.minstrel.cache.db.dao.DiagnosticEventDao import com.fabledsword.minstrel.cache.db.dao.CachedPlaylistDao import com.fabledsword.minstrel.cache.db.dao.CachedPlaylistTrackDao import com.fabledsword.minstrel.cache.db.dao.CachedQuarantineDao @@ -105,5 +106,10 @@ object DatabaseModule { fun provideAudioCacheIndexDao(db: AppDatabase): AudioCacheIndexDao = db.audioCacheIndexDao() + @Provides + @Singleton + fun provideDiagnosticEventDao(db: AppDatabase): DiagnosticEventDao = + db.diagnosticEventDao() + private const val DATABASE_NAME = "minstrel.db" } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/AuthSessionDao.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/AuthSessionDao.kt index 5bb2ccc9..3df6b01c 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/AuthSessionDao.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/AuthSessionDao.kt @@ -42,4 +42,8 @@ interface AuthSessionDao { /** Partial update: change only the serialized cache settings. */ @Query("UPDATE auth_session SET cacheSettingsJson = :json WHERE id = 0") suspend fun setCacheSettingsJson(json: String?) + + /** Partial update: change only the per-device diagnostics opt-out. */ + @Query("UPDATE auth_session SET diagnosticsOptOut = :optOut WHERE id = 0") + suspend fun setDiagnosticsOptOut(optOut: Boolean) } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/DiagnosticEventDao.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/DiagnosticEventDao.kt new file mode 100644 index 00000000..2361fc48 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/DiagnosticEventDao.kt @@ -0,0 +1,35 @@ +package com.fabledsword.minstrel.cache.db.dao + +import androidx.room.Dao +import androidx.room.Insert +import androidx.room.Query +import com.fabledsword.minstrel.cache.db.entities.DiagnosticEventEntity + +@Dao +interface DiagnosticEventDao { + @Insert + suspend fun insert(row: DiagnosticEventEntity): Long + + /** FIFO drain order so the uploader sends oldest-first. */ + @Query("SELECT * FROM diagnostic_events ORDER BY id ASC LIMIT :limit") + suspend fun takeBatch(limit: Int): List + + @Query("SELECT COUNT(*) FROM diagnostic_events") + suspend fun count(): Int + + @Query("DELETE FROM diagnostic_events WHERE id IN (:ids)") + suspend fun deleteByIds(ids: List) + + /** + * Ring-buffer trim: drop the oldest rows beyond [keep]. Called after + * insert so a long offline stretch can't grow the buffer unbounded. + */ + @Query( + "DELETE FROM diagnostic_events WHERE id NOT IN " + + "(SELECT id FROM diagnostic_events ORDER BY id DESC LIMIT :keep)", + ) + suspend fun trimToNewest(keep: Int) + + @Query("DELETE FROM diagnostic_events") + suspend fun clear() +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/entities/AuthSessionEntity.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/entities/AuthSessionEntity.kt index c36cfa34..6b8456c9 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/entities/AuthSessionEntity.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/entities/AuthSessionEntity.kt @@ -36,4 +36,11 @@ data class AuthSessionEntity( * CacheSettings shape evolves. */ val cacheSettingsJson: String? = null, + /** + * Per-device opt-out of diagnostics reporting (M9). When the + * account's debug mode is enabled by an admin, the user can still + * turn reporting OFF on this device (battery/privacy) — that local + * choice lives here. Default false = honor the account flag. + */ + val diagnosticsOptOut: Boolean = false, ) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/entities/DiagnosticEventEntity.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/entities/DiagnosticEventEntity.kt new file mode 100644 index 00000000..7e5f2868 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/entities/DiagnosticEventEntity.kt @@ -0,0 +1,28 @@ +package com.fabledsword.minstrel.cache.db.entities + +import androidx.room.Entity +import androidx.room.PrimaryKey + +/** + * One buffered device-diagnostics event (M9). The DiagnosticsReporter + * writes rows here when the account's debug mode is on; the + * DiagnosticsUploader drains them to POST /api/diagnostics and deletes + * on success. + * + * This is a deliberate ring buffer that is NOT routed through the offline + * MutationQueue: diagnostics are high-volume, best-effort telemetry, and + * the bug we're chasing (roaming / dead-zone recovery) happens WHILE + * offline — so events must persist locally through the dead zone and + * upload on recovery, without clogging the user-data mutation replay. + * + * `occurredAtMillis` is the device-clock epoch-ms when the event happened + * (the server also stamps its own received_at). `payloadJson` is an + * opaque JSON object carrying the event sub-type + fields. + */ +@Entity(tableName = "diagnostic_events") +data class DiagnosticEventEntity( + @PrimaryKey(autoGenerate = true) val id: Long = 0, + val kind: String, + val occurredAtMillis: Long, + val payloadJson: String, +) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt new file mode 100644 index 00000000..bb743ff7 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt @@ -0,0 +1,284 @@ +package com.fabledsword.minstrel.diagnostics + +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.content.IntentFilter +import android.os.PowerManager +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleOwner +import androidx.core.content.ContextCompat +import androidx.lifecycle.ProcessLifecycleOwner +import com.fabledsword.minstrel.auth.AuthController +import com.fabledsword.minstrel.auth.AuthStore +import com.fabledsword.minstrel.cache.db.dao.DiagnosticEventDao +import com.fabledsword.minstrel.cache.db.entities.DiagnosticEventEntity +import com.fabledsword.minstrel.connectivity.ConnectivityObserver +import com.fabledsword.minstrel.connectivity.NetworkStatusController +import com.fabledsword.minstrel.di.ApplicationScope +import com.fabledsword.minstrel.player.PlayerController +import com.fabledsword.minstrel.player.RemotePlayerState +import com.fabledsword.minstrel.player.output.OutputPickerController +import com.fabledsword.minstrel.player.output.OutputRoute +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.launch +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import javax.inject.Inject +import javax.inject.Singleton + +/** + * On-device diagnostics collector (M9). Active only while the account's + * debug mode is enabled (server flag, read via AuthController.currentUser) + * AND the user hasn't opted out locally. When active it taps existing + * connectivity / player / UPnP / power seams READ-ONLY and writes a + * timeseries of events into the [DiagnosticEventDao] ring buffer; the + * [DiagnosticsUploader] drains that buffer to the server. + * + * The aim is to explain two failure classes after the fact: roaming / + * poor-data recovery (connectivity + server-health transitions) and + * locked-phone UPnP desync (power/Doze transitions correlated with the + * Sonos-vs-local position deltas captured in the heartbeat). + */ +@Suppress("TooManyFunctions") // one collector + one (un)register pair per signal; cohesive +@Singleton +class DiagnosticsReporter @Inject constructor( + @ApplicationContext private val context: Context, + @ApplicationScope private val scope: CoroutineScope, + private val dao: DiagnosticEventDao, + private val authController: AuthController, + private val authStore: AuthStore, + private val connectivity: ConnectivityObserver, + private val networkStatus: NetworkStatusController, + private val playerController: PlayerController, + private val outputPicker: OutputPickerController, + private val remoteState: RemotePlayerState, + private val json: Json, +) { + @Volatile private var enabled = false + private var collectorJob: Job? = null + private var powerReceiver: BroadcastReceiver? = null + private var lifecycleObserver: DefaultLifecycleObserver? = null + + init { + // Gate = account flag (currentUser.debugModeEnabled) AND not the + // per-device local opt-out. + scope.launch { + combine(authController.currentUser, authStore.diagnosticsOptOut) { user, optOut -> + (user?.debugModeEnabled == true) && !optOut + }.distinctUntilChanged().collect { setEnabled(it) } + } + // Periodically re-pull /me so a remote admin flip propagates + // without a re-login or app restart. + scope.launch { + while (true) { + delay(GATE_REFRESH_MS) + authController.refreshProfile() + } + } + } + + private fun setEnabled(value: Boolean) { + if (value == enabled) return + enabled = value + if (value) start() else stop() + } + + private fun start() { + registerPowerReceiver() + registerLifecycleObserver() + collectorJob = scope.launch { + record("lifecycle", buildJsonObject { put("event", "reporting_started") }) + launch { collectConnectivity() } + launch { collectServerHealth() } + launch { collectUpnpDrops() } + launch { collectPlayerState() } + launch { collectRoutes() } + launch { heartbeatLoop() } + } + } + + private fun stop() { + collectorJob?.cancel() + collectorJob = null + unregisterPowerReceiver() + unregisterLifecycleObserver() + // Off means off: drop any unsent buffer (honors the user OFF + // intent and avoids leaking data captured before disable). + scope.launch { runCatching { dao.clear() } } + } + + private suspend fun collectConnectivity() { + connectivity.online.distinctUntilChanged().collect { online -> + record("connectivity", buildJsonObject { + put("event", "online_changed") + put("online", online) + }) + } + } + + private suspend fun collectServerHealth() { + networkStatus.state.distinctUntilChanged().collect { s -> + record("connectivity", buildJsonObject { + put("event", "server_health") + put("state", s.name) + }) + } + } + + private suspend fun collectUpnpDrops() { + playerController.dropEvents.collect { msg -> + record("upnp_sync", buildJsonObject { + put("event", "drop") + put("message", msg) + }) + } + } + + private suspend fun collectPlayerState() { + playerController.uiState + .map { Triple(it.currentSource, it.isUpnpLoading, it.playbackError) } + .distinctUntilChanged() + .collect { (source, upnpLoading, err) -> + record("upnp_sync", buildJsonObject { + put("event", "player_state") + put("source", source ?: "") + put("upnp_loading", upnpLoading) + if (err != null) put("error", err) + }) + } + } + + private suspend fun collectRoutes() { + outputPicker.routesState.map { it.current }.distinctUntilChanged().collect { r -> + record("upnp_sync", buildJsonObject { + put("event", "route") + put("id", r.id) + put("name", r.name) + put("kind", r.kind.name) + put("protocol", r.protocol.name) + put("connected", r.isConnected) + }) + } + } + + private suspend fun heartbeatLoop() { + while (true) { + delay(HEARTBEAT_MS) + val ui = playerController.uiState.value + val route = outputPicker.routesState.value.current + val routeActive = route.protocol != OutputRoute.Protocol.SYSTEM + // Idle = nothing worth sampling. Skip to keep the buffer lean. + if (!ui.isPlaying && !routeActive) continue + record("heartbeat", buildJsonObject { + put("server_health", networkStatus.state.value.name) + put("is_playing", ui.isPlaying) + put("source", ui.currentSource ?: "") + put("local_index", ui.queueIndex) + put("local_pos_ms", ui.positionMs) + put("route", route.name) + put("route_protocol", route.protocol.name) + // UPnP/Sonos remote-vs-local — the desync signal. + put("sonos_track", remoteState.trackNumber) + put("sonos_pos_ms", remoteState.positionMs) + put("sonos_playing", remoteState.isPlaying) + addPowerFields(this) + }) + } + } + + // --- power + lifecycle taps ------------------------------------------- + + private fun registerPowerReceiver() { + if (powerReceiver != null) return + val receiver = object : BroadcastReceiver() { + override fun onReceive(c: Context?, intent: Intent?) { + val action = intent?.action ?: return + if (!enabled) return + scope.launch { record("power", powerEvent(action)) } + } + } + val filter = IntentFilter().apply { + addAction(Intent.ACTION_SCREEN_ON) + addAction(Intent.ACTION_SCREEN_OFF) + addAction(PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED) + } + // System-protected broadcasts, but pass NOT_EXPORTED explicitly so + // the API 34+ registerReceiver flag requirement is satisfied. + ContextCompat.registerReceiver( + context, receiver, filter, ContextCompat.RECEIVER_NOT_EXPORTED, + ) + powerReceiver = receiver + } + + private fun unregisterPowerReceiver() { + powerReceiver?.let { runCatching { context.unregisterReceiver(it) } } + powerReceiver = null + } + + private fun registerLifecycleObserver() { + if (lifecycleObserver != null) return + val obs = object : DefaultLifecycleObserver { + override fun onStart(owner: LifecycleOwner) = logLifecycle("app_foreground") + override fun onStop(owner: LifecycleOwner) = logLifecycle("app_background") + } + lifecycleObserver = obs + scope.launch(Dispatchers.Main) { + ProcessLifecycleOwner.get().lifecycle.addObserver(obs) + } + } + + private fun unregisterLifecycleObserver() { + val obs = lifecycleObserver ?: return + lifecycleObserver = null + scope.launch(Dispatchers.Main) { + ProcessLifecycleOwner.get().lifecycle.removeObserver(obs) + } + } + + private fun logLifecycle(event: String) { + if (!enabled) return + scope.launch { record("lifecycle", buildJsonObject { put("event", event) }) } + } + + private fun powerEvent(action: String): JsonObject = buildJsonObject { + put("event", action.substringAfterLast('.')) + addPowerFields(this) + } + + private fun addPowerFields(builder: kotlinx.serialization.json.JsonObjectBuilder) { + val pm = context.getSystemService(Context.POWER_SERVICE) as PowerManager + builder.put("doze", pm.isDeviceIdleMode) + builder.put("screen_on", pm.isInteractive) + builder.put("battery_opt_ignored", pm.isIgnoringBatteryOptimizations(context.packageName)) + } + + private suspend fun record(kind: String, payload: JsonObject) { + runCatching { + val id = dao.insert( + DiagnosticEventEntity( + kind = kind, + occurredAtMillis = System.currentTimeMillis(), + payloadJson = json.encodeToString(JsonObject.serializer(), payload), + ), + ) + if (id % TRIM_INTERVAL == 0L) dao.trimToNewest(MAX_BUFFER) + } + } + + private companion object { + const val HEARTBEAT_MS = 45_000L + const val GATE_REFRESH_MS = 300_000L // re-pull /me every 5 min + const val MAX_BUFFER = 5_000 // ring-buffer ceiling, oldest dropped + const val TRIM_INTERVAL = 50L // trim every N inserts, not every one + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsUploader.kt b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsUploader.kt new file mode 100644 index 00000000..6ed816b2 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsUploader.kt @@ -0,0 +1,115 @@ +package com.fabledsword.minstrel.diagnostics + +import android.os.Build +import com.fabledsword.minstrel.BuildConfig +import com.fabledsword.minstrel.api.endpoints.DiagnosticEventWire +import com.fabledsword.minstrel.api.endpoints.DiagnosticsApi +import com.fabledsword.minstrel.api.endpoints.DiagnosticsReportRequest +import com.fabledsword.minstrel.auth.AuthStore +import com.fabledsword.minstrel.cache.db.dao.DiagnosticEventDao +import com.fabledsword.minstrel.cache.db.entities.DiagnosticEventEntity +import com.fabledsword.minstrel.connectivity.NetworkStatusController +import com.fabledsword.minstrel.connectivity.ServerHealth +import com.fabledsword.minstrel.di.ApplicationScope +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.serialization.json.Json +import retrofit2.Retrofit +import retrofit2.create +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Drains the [DiagnosticEventDao] ring buffer to POST /api/diagnostics + * (M9). Triggers mirror [com.fabledsword.minstrel.cache.mutations.MutationReplayer]: + * a periodic tick, a sign-in (sessionCookie non-null), and a server-health + * recovery to [ServerHealth.Healthy] — so a dead-zone's buffered events + * upload the moment the link comes back. + * + * Deliberately NOT the MutationQueue: diagnostics are high-volume, + * best-effort telemetry. A failed upload just retries on the next tick; + * the server no-ops (204) when the account's debug flag is off, which the + * uploader treats as success so the buffer drains either way. + */ +@Singleton +class DiagnosticsUploader @Inject constructor( + private val dao: DiagnosticEventDao, + private val authStore: AuthStore, + private val json: Json, + @ApplicationScope private val scope: CoroutineScope, + networkStatus: NetworkStatusController, + retrofit: Retrofit, +) { + private val api: DiagnosticsApi = retrofit.create() + private val mutex = Mutex() + + init { + scope.launch { + while (true) { + delay(UPLOAD_INTERVAL_MS) + drainSafe() + } + } + scope.launch { + authStore.sessionCookie.filterNotNull().distinctUntilChanged().collect { drainSafe() } + } + scope.launch { + networkStatus.state + .map { it == ServerHealth.Healthy } + .distinctUntilChanged() + .filter { it } + .collect { drainSafe() } + } + } + + /** Coalesces concurrent triggers via tryLock — a drain in flight wins. */ + private suspend fun drainSafe() { + if (authStore.sessionCookie.value.isNullOrEmpty()) return + val clientId = authStore.clientId.value ?: return + if (!mutex.tryLock()) return + try { + drain(clientId) + } finally { + mutex.unlock() + } + } + + private suspend fun drain(clientId: String) { + while (true) { + val batch = runCatching { dao.takeBatch(BATCH_SIZE) }.getOrNull().orEmpty() + if (batch.isEmpty()) return + val sent = runCatching { upload(clientId, batch) }.isSuccess + if (!sent) return // leave rows for the next trigger + runCatching { dao.deleteByIds(batch.map { it.id }) } + if (batch.size < BATCH_SIZE) return + } + } + + private suspend fun upload(clientId: String, batch: List) { + api.report( + DiagnosticsReportRequest( + clientId = clientId, + appVersion = BuildConfig.VERSION_NAME, + osVersion = "Android ${Build.VERSION.RELEASE} (${Build.MODEL})", + events = batch.map { row -> + DiagnosticEventWire( + kind = row.kind, + occurredAt = row.occurredAtMillis, + payload = json.parseToJsonElement(row.payloadJson), + ) + }, + ), + ) + } + + private companion object { + const val UPLOAD_INTERVAL_MS = 60_000L + const val BATCH_SIZE = 100 + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/UserRef.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/UserRef.kt index 35572695..e527451c 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/UserRef.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/UserRef.kt @@ -16,4 +16,11 @@ data class UserRef( val id: String, val username: String, val isAdmin: Boolean = false, + /** + * Account-level diagnostics opt-in (M9). NOT present in the login + * response (that's the narrow UserView) — populated by the /api/me + * refresh in AuthController so a remote admin flip reaches the + * device. Default false until the first refresh. + */ + val debugModeEnabled: Boolean = false, ) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/MyProfileWire.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/MyProfileWire.kt index 7c811efc..0086bb3e 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/MyProfileWire.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/MyProfileWire.kt @@ -18,4 +18,5 @@ data class MyProfileWire( @SerialName("display_name") val displayName: String? = null, val email: String? = null, @SerialName("is_admin") val isAdmin: Boolean = false, + @SerialName("debug_mode_enabled") val debugModeEnabled: Boolean = false, ) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/settings/ui/SettingsScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/settings/ui/SettingsScreen.kt index 943b723e..72e9fb91 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/settings/ui/SettingsScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/settings/ui/SettingsScreen.kt @@ -28,6 +28,7 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.SegmentedButton import androidx.compose.material3.SegmentedButtonDefaults import androidx.compose.material3.SingleChoiceSegmentedButtonRow +import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable @@ -94,6 +95,7 @@ fun SettingsScreen( onPickTheme = themeVm::setThemeMode, onNavToRequests = { navController.navigate(Requests) }, onNavToAdmin = { navController.navigate(Admin) }, + onToggleDiagnostics = viewModel::setDiagnosticsOptOut, onSignOutClick = { showSignOutConfirm = true }, ) } @@ -116,6 +118,7 @@ private fun SettingsList( onPickTheme: (ThemeMode) -> Unit, onNavToRequests: () -> Unit, onNavToAdmin: () -> Unit, + onToggleDiagnostics: (Boolean) -> Unit, onSignOutClick: () -> Unit, ) { Column( @@ -148,6 +151,12 @@ private fun SettingsList( ProfileCard() PasswordCard() ListenBrainzCard() + if (state.diagnosticsEnabledByAdmin) { + DiagnosticsCard( + optOut = state.diagnosticsOptOut, + onToggle = onToggleDiagnostics, + ) + } AppearanceCard(themeMode = themeMode, onPick = onPickTheme) StorageCard() AboutCard() @@ -262,6 +271,46 @@ private fun AccountCard(username: String, isAdmin: Boolean, serverUrl: String) { } } +// Shown only while an admin has enabled debug mode on this account. Lets +// the user turn reporting OFF on this device (battery/privacy) without +// touching the account flag. Checked = reporting on = NOT opted out. +@Composable +private fun DiagnosticsCard(optOut: Boolean, onToggle: (Boolean) -> Unit) { + ElevatedCard(modifier = Modifier.fillMaxWidth()) { + Column( + modifier = Modifier.padding(16.dp), + verticalArrangement = Arrangement.spacedBy(8.dp), + ) { + Text( + text = "Diagnostics", + style = MaterialTheme.typography.titleMedium, + color = MaterialTheme.colorScheme.onSurface, + ) + Row(verticalAlignment = Alignment.CenterVertically) { + Column(modifier = Modifier.weight(1f)) { + Text( + text = "Send diagnostic reports", + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface, + ) + Text( + text = "Your admin enabled debug mode to investigate an " + + "issue. This device streams connectivity and playback " + + "diagnostics. Turn off any time.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + Spacer(Modifier.size(12.dp)) + Switch( + checked = !optOut, + onCheckedChange = { checked -> onToggle(!checked) }, + ) + } + } + } +} + @OptIn(ExperimentalMaterial3Api::class) @Composable private fun AppearanceCard(themeMode: ThemeMode, onPick: (ThemeMode) -> Unit) { diff --git a/android/app/src/main/java/com/fabledsword/minstrel/settings/ui/SettingsViewModel.kt b/android/app/src/main/java/com/fabledsword/minstrel/settings/ui/SettingsViewModel.kt index ebe2f563..256ad581 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/settings/ui/SettingsViewModel.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/settings/ui/SettingsViewModel.kt @@ -19,12 +19,15 @@ data class SettingsState( val isAdmin: Boolean = false, val isSigningOut: Boolean = false, val signedOut: Boolean = false, + // M9: account debug flag (admin-set) + this device's local opt-out. + val diagnosticsEnabledByAdmin: Boolean = false, + val diagnosticsOptOut: Boolean = false, ) @HiltViewModel class SettingsViewModel @Inject constructor( private val authController: AuthController, - authStore: AuthStore, + private val authStore: AuthStore, ) : ViewModel() { private val transient = MutableStateFlow(TransientState()) @@ -41,19 +44,27 @@ class SettingsViewModel @Inject constructor( combine( authStore.baseUrl, authController.currentUser, + authStore.diagnosticsOptOut, transient, - ) { url, user, t -> + ) { url, user, optOut, t -> SettingsState( serverUrl = url, username = user?.username.orEmpty(), isAdmin = user?.isAdmin == true, isSigningOut = t.isSigningOut, signedOut = t.signedOut, + diagnosticsEnabledByAdmin = user?.debugModeEnabled == true, + diagnosticsOptOut = optOut, ) }.collect { internal.value = it } } } + /** Per-device opt-out of diagnostics while the account flag is on. */ + fun setDiagnosticsOptOut(optOut: Boolean) { + authStore.setDiagnosticsOptOut(optOut) + } + fun signOut() { if (transient.value.isSigningOut) return viewModelScope.launch { diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index 9533a9e9..8e4d775a 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -391,6 +391,7 @@ export type AdminUser = { display_name: string | null; is_admin: boolean; auto_approve_requests: boolean; + debug_mode_enabled: boolean; created_at: string; }; @@ -451,6 +452,12 @@ export async function updateUserAutoApprove(id: string, autoApprove: boolean): P return api.put(`/api/admin/users/${id}/auto-approve`, { auto_approve: autoApprove }); } +// Flip an account's diagnostics/debug-reporting opt-in (M9). Admin-set; +// the client obeys it (with a local per-device OFF switch). +export async function updateUserDebugMode(id: string, enabled: boolean): Promise { + return api.put(`/api/admin/users/${id}/debug-mode`, { enabled }); +} + export function createAdminUsersQuery() { return createQuery({ queryKey: qk.adminUsers(), @@ -497,3 +504,82 @@ export function createSMTPConfigQuery() { staleTime: 60_000 }); } + +// Device diagnostics (M9) --------------------------------------------------- + +// Coarse event category. The finer event sub-type lives inside `payload`. +export type DiagnosticKind = + | 'connectivity' + | 'upnp_sync' + | 'power' + | 'lifecycle' + | 'heartbeat' + | 'http'; + +export type AdminDiagnostic = { + id: string; + user_id: string; + username: string; + client_id: string; + app_version?: string; + os_version?: string; + kind: DiagnosticKind | string; + payload: Record; + occurred_at: string; + received_at: string; +}; + +export type AdminDiagnosticDevice = { + client_id: string; + user_id: string; + username: string; + app_version: string; + os_version: string; + last_seen: string; + event_count: number; +}; + +export type DiagnosticsFilter = { + userId?: string; + clientId?: string; + kind?: string; + from?: string; // RFC3339 + to?: string; // RFC3339 + limit?: number; +}; + +export async function listAdminDiagnostics(f: DiagnosticsFilter): Promise { + const params = new URLSearchParams(); + if (f.userId) params.set('user_id', f.userId); + if (f.clientId) params.set('client_id', f.clientId); + if (f.kind) params.set('kind', f.kind); + if (f.from) params.set('from', f.from); + if (f.to) params.set('to', f.to); + if (f.limit !== undefined) params.set('limit', String(f.limit)); + const qs = params.toString(); + return api.get(qs ? `/api/admin/diagnostics?${qs}` : '/api/admin/diagnostics'); +} + +export async function listDiagnosticDevices(userId?: string): Promise { + const qs = userId ? `?user_id=${userId}` : ''; + return api.get(`/api/admin/diagnostics/devices${qs}`); +} + +export function createAdminDiagnosticsQuery(f: DiagnosticsFilter) { + return createQuery({ + queryKey: qk.adminDiagnostics(f as Record), + queryFn: () => listAdminDiagnostics(f), + // The operator enables debug then watches events stream in; a short + // poll keeps the timeline live without manual refresh. + refetchInterval: 10_000, + staleTime: 5_000 + }); +} + +export function createDiagnosticDevicesQuery(userId?: string) { + return createQuery({ + queryKey: qk.adminDiagnosticDevices(userId), + queryFn: () => listDiagnosticDevices(userId), + staleTime: 15_000 + }); +} diff --git a/web/src/lib/api/queries.ts b/web/src/lib/api/queries.ts index 71f444f4..d12397e1 100644 --- a/web/src/lib/api/queries.ts +++ b/web/src/lib/api/queries.ts @@ -50,6 +50,10 @@ export const qk = { coverProviders: () => ['coverProviders'] as const, adminUsers: () => ['adminUsers'] as const, adminInvites: () => ['adminInvites'] as const, + adminDiagnostics: (f: Record) => + ['adminDiagnostics', f] as const, + adminDiagnosticDevices: (userId?: string) => + ['adminDiagnosticDevices', { userId: userId ?? 'all' }] as const, smtpConfig: () => ['smtpConfig'] as const, suggestions: (limit?: number) => ['suggestions', { limit: limit ?? 12 }] as const, diff --git a/web/src/lib/components/AdminTabs.svelte b/web/src/lib/components/AdminTabs.svelte index 78ad4cc6..f7acbabd 100644 --- a/web/src/lib/components/AdminTabs.svelte +++ b/web/src/lib/components/AdminTabs.svelte @@ -9,6 +9,7 @@ { href: '/admin/requests', label: 'Requests' }, { href: '/admin/quarantine', label: 'Quarantine' }, { href: '/admin/playback-errors', label: 'Playback errors' }, + { href: '/admin/diagnostics', label: 'Diagnostics' }, { href: '/admin/users', label: 'Users' } ]; diff --git a/web/src/routes/admin/diagnostics/+page.svelte b/web/src/routes/admin/diagnostics/+page.svelte new file mode 100644 index 00000000..f6fb0e34 --- /dev/null +++ b/web/src/routes/admin/diagnostics/+page.svelte @@ -0,0 +1,302 @@ + + +{pageTitle('Admin · Diagnostics')} + +
+
+

Device diagnostics

+

+ Enable debug mode on an account to have its device(s) stream a timeseries + of connectivity, UPnP-sync, and power/Doze events here. Filter to the + window you care about, then copy or download the slice for analysis. +

+
+ + +
+
+ + + {#if selectedUser} + + + + {selectedUser.debug_mode_enabled ? 'Reporting active' : 'Reporting off'} + + {/if} +
+
+ + +
+ + + + + + +
+ + +
+
+ + + {#if diagQuery.isError} +

Couldn't load: {errMessage(diagQuery.error)}

+ {:else if diagQuery.isPending} +

Loading…

+ {:else if rows.length === 0} +

+ No events for this filter. Enable debug mode on an account and have the + device reproduce the issue — events appear here within a minute. +

+ {:else} +
+ {rows.length} events (oldest first) +
+
    + {#each rows as r (r.id)} +
  • + + {fmtTime(r.occurred_at)} + + + {kindLabel(r.kind)} + +
    + + {payloadPreview(r.payload)} + + 
    {JSON.stringify(r.payload, null, 2)}
    +
    +
    • + {/each} +
+ {/if} +
From 8a58f07237b31c5ba6e3e2a58f23532dc4bb4d15 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 29 Jun 2026 18:59:05 -0400 Subject: [PATCH 3/5] fix(android/diagnostics): keep uploader drain within ReturnCount gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drainSafe/drain each had 3 returns (detekt ReturnCount ≤ 2). Collapse the guard clauses and convert drain's loop to a `more` flag — same behavior, zero/two returns. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- .../diagnostics/DiagnosticsUploader.kt | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsUploader.kt b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsUploader.kt index 6ed816b2..b4be91c6 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsUploader.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsUploader.kt @@ -70,8 +70,10 @@ class DiagnosticsUploader @Inject constructor( /** Coalesces concurrent triggers via tryLock — a drain in flight wins. */ private suspend fun drainSafe() { - if (authStore.sessionCookie.value.isNullOrEmpty()) return - val clientId = authStore.clientId.value ?: return + val clientId = authStore.clientId.value + // Signed out / no client id yet → nothing to do. Single guard keeps + // the return count within the detekt gate. + if (authStore.sessionCookie.value.isNullOrEmpty() || clientId == null) return if (!mutex.tryLock()) return try { drain(clientId) @@ -81,13 +83,14 @@ class DiagnosticsUploader @Inject constructor( } private suspend fun drain(clientId: String) { - while (true) { + var more = true + while (more) { val batch = runCatching { dao.takeBatch(BATCH_SIZE) }.getOrNull().orEmpty() - if (batch.isEmpty()) return - val sent = runCatching { upload(clientId, batch) }.isSuccess - if (!sent) return // leave rows for the next trigger - runCatching { dao.deleteByIds(batch.map { it.id }) } - if (batch.size < BATCH_SIZE) return + val sent = batch.isNotEmpty() && runCatching { upload(clientId, batch) }.isSuccess + if (sent) runCatching { dao.deleteByIds(batch.map { it.id }) } + // Keep going only while a full batch sent cleanly; otherwise stop + // (empty buffer, or a failure to retry on the next trigger). + more = sent && batch.size >= BATCH_SIZE } } From bffa5b28bd5f19334a3e93ddfae3f990673afc22 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 29 Jun 2026 19:05:50 -0400 Subject: [PATCH 4/5] fix(diagnostics): StateFlow distinctUntilChanged build error + AdminUser test fixtures - DiagnosticsReporter.collectServerHealth: drop distinctUntilChanged() on networkStatus.state (StateFlow is already distinct; the deprecation warning is a hard error under allWarningsAsErrors). - web users.test.ts: add debug_mode_enabled to the alice/bob AdminUser fixtures now that the field is required on the type. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- .../fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt | 4 +++- web/src/routes/admin/users/users.test.ts | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt index bb743ff7..fe827015 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt @@ -127,7 +127,9 @@ class DiagnosticsReporter @Inject constructor( } private suspend fun collectServerHealth() { - networkStatus.state.distinctUntilChanged().collect { s -> + // state is a StateFlow — already conflated/distinct, so no + // distinctUntilChanged (it's a deprecation warning = build error). + networkStatus.state.collect { s -> record("connectivity", buildJsonObject { put("event", "server_health") put("state", s.name) diff --git a/web/src/routes/admin/users/users.test.ts b/web/src/routes/admin/users/users.test.ts index 08b641b0..8869a2d7 100644 --- a/web/src/routes/admin/users/users.test.ts +++ b/web/src/routes/admin/users/users.test.ts @@ -38,6 +38,7 @@ const alice: AdminUser = { display_name: null, is_admin: true, auto_approve_requests: false, + debug_mode_enabled: false, created_at: '2026-05-01T00:00:00Z' }; @@ -47,6 +48,7 @@ const bob: AdminUser = { display_name: 'Bob B', is_admin: false, auto_approve_requests: false, + debug_mode_enabled: false, created_at: '2026-05-02T00:00:00Z' }; From 782f152d3723aa49dddfe71f624eb4b214f067bc Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 29 Jun 2026 19:11:39 -0400 Subject: [PATCH 5/5] test(web/admin): AdminTabs now has seven tabs (Diagnostics added) Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- web/src/lib/components/AdminTabs.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web/src/lib/components/AdminTabs.test.ts b/web/src/lib/components/AdminTabs.test.ts index 73bd812f..2163c178 100644 --- a/web/src/lib/components/AdminTabs.test.ts +++ b/web/src/lib/components/AdminTabs.test.ts @@ -52,7 +52,7 @@ describe('AdminTabs', () => { ); }); - test('renders all six tabs in order', () => { + test('renders all seven tabs in order', () => { state.pageUrl = new URL('http://localhost/admin'); render(AdminTabs); const links = screen.getAllByRole('link'); @@ -62,6 +62,7 @@ describe('AdminTabs', () => { 'Requests', 'Quarantine', 'Playback errors', + 'Diagnostics', 'Users' ]); });