Commit Graph
7 Commits
Author SHA1 Message Date
f235524365 feat(diagnostics): chunked report upload fallback for proxy body caps (#494)
* feat(diagnostics): chunked report upload fallback for proxy body caps

Diagnostics bundles can be up to max_bundle_bytes (10 MiB default), but a
reverse proxy in front of Silo commonly caps request bodies at nginx's
default client_max_body_size of 1 MiB. Such a proxy answers the single-shot
multipart upload with its own 413 before Silo ever sees the request, so any
report over the cap could never be delivered.

Add a chunked upload fallback under /api/v1/diagnostics/reports/uploads:

- POST   /                      {manifest, bundle_bytes} opens a session
- PUT    /{id}/chunks/{index}   streams one ≤768 KiB chunk (proxy-safe)
- POST   /{id}/complete         ingests the assembled bundle
- DELETE /{id}                  best-effort abandon

The assembled bundle goes through the exact same Ingest path as the
single-shot endpoint, so every content check (manifest contract, archive
sha/bytes/entries, quotas, profile attribution) applies identically.
Sessions reuse internal/uploads (the plugin chunked-upload spool manager)
plus a small owner map for per-user isolation; they spool to disk, expire
after 15 minutes, cap at one per user / 16 global, and complete shares the
existing per-user + global in-flight ingest limiter.

/diagnostics/status now advertises upload_chunk_bytes so clients can detect
support; older servers omit the field and clients treat that as
unsupported. The demo guard's diagnostics prefix gains PUT to cover the
chunk route.

Verified end to end against an OpenResty proxy with a 1m body cap: the
single-shot upload 413s, the same 1.6 MiB bundle uploads in three chunks
and lands as an accepted report; also exercised from the tvOS client's
fallback path in the simulator.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(diagnostics): harden chunked upload sessions per review

- Reserve the per-user slot and global cap atomically in init (a
  reservation map counted with live sessions), so concurrent inits by one
  account can no longer fan out past one session or transiently exceed the
  cap. Creation failures roll the reservation back.
- Move chunk body I/O outside the uploads.Manager mutex: a slow client
  streaming one chunk no longer serializes every other session's chunk
  writes, completes, and cancels. A per-chunk in-flight flag rejects
  duplicate concurrent writes to the same offset (ErrChunkBusy → 409), and
  cancel/expiry defer spool-directory removal to the last finishing
  writer.
- Chunk arrivals refresh the session expiry, making the TTL an idle
  timeout instead of an absolute deadline so a slow-but-progressing upload
  cannot expire mid-transfer.
- Extend the request read deadline on chunk PUTs and both deadlines on
  complete, matching the single-shot handler's slow-uplink handling.
- Keep the session when complete's availability re-check fails
  transiently (status load error → 500): only definitive
  disabled/storage-unavailable answers discard the spool, so a retried
  complete succeeds without re-uploading every chunk.
- Reclaim orphaned spool directories at startup (a restart previously
  stranded the old process's partial uploads forever) and sweep expired
  sessions on a timer instead of only from later init traffic.
- Document that session state is process-local and what that means for
  multi-replica deployments.

Adds concurrency/race tests (go test -race) for atomic admission,
same-chunk write exclusion, expiry refresh, transient-status retry, and
startup reclaim.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(diagnostics): count detached chunk writers and lift chunk PUT write deadline

Second review round:

- A canceled session whose slow chunk writer was still draining held a
  connection and spool disk but vanished from every count, so a
  cancel-and-reinit loop could stack unbounded live writers behind the
  16-session cap. The uploads manager now parks such sessions in a
  detached set (exposed as DetachedWriterSessions) until their last
  writer returns, and diagnostics init counts them in its admission gate.
- Chunk PUTs now extend the write deadline as well as the read deadline:
  on an uplink slow enough to eat the server's 120s WriteTimeout, the
  stored chunk's JSON acknowledgement would otherwise be lost and the
  client would retry an already-accepted chunk.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-27 12:07:37 -04:00
Quick104andClaude Fable 5 9fad08a6fa fix(diagnostics): address round-6 review findings on PR #445
- AdminDiagnostics list: fix regression where rows dereferenced the
  now-omitted manifest for app_build. Project app_build server-side out
  of manifest JSONB into both list and detail responses (cheap
  COALESCE(manifest->'report'->>'app_build','')), split the TS type into
  DiagnosticReportSummary (list, no manifest) and DiagnosticReport
  (detail, with manifest), and read report.app_build in the row/detail.
- embeddedManifestMatches: decode with json.Decoder + UseNumber so large
  integers above 2^53 (e.g. log_summary.lines) can't collapse to the same
  float and falsely match; re-assert no-trailing-data strictness.
- Quota reservation (SKIP): reserving the client-claimed archive.bytes is
  sound because archiveMatches requires claimed==actual before MarkReady,
  so no stored report exceeds its reservation; documented in a code comment.
- Multipart parts: reject a wrong-name/wrong-content-type part without
  calling part.Close(), which would drain up to the bundle limit while
  holding the in-flight slot; abandon it so malformed uploads fail promptly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
2026-07-21 14:23:59 -04:00
Quick104andClaude Fable 5 dee46f9398 fix(diagnostics): address round-5 review findings on PR #445
- service: reject supplied child-profile attribution with a distinct
  ErrChildProfileForbidden (403 child_profile_forbidden) instead of
  silently dropping it as if the profile were not found; a profile that
  is simply not the user's still drops attribution unchanged
- repo: add a manifest-free list projection (reportListSelectSQL /
  scanReportSummary) for admin list and retention/stale cleanup queries
  so they no longer drag the full manifest JSONB per row; keep the full
  projection for GetByID/DeleteByID and mark Manifest omitempty
- cleanup: delete/mark the DB row before the blob in retention and stale
  loops so a mid-run DB failure can't leave a ready report pointing at a
  missing bundle; blob-delete failures are logged with bucket/keys for
  orphan cleanup to reap rather than aborting the run (shared helper with
  the admin DeleteReport path)
- admin: reject diagnostics settings where max_bytes_per_user would fall
  below max_bundle_bytes (and the reciprocal), which would make every
  max-size upload fail quota
- router/demo: route POST /diagnostics/reports through DemoGuard and block
  the reports prefix in demo mode while keeping GET /diagnostics/status
  available

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
2026-07-21 13:36:08 -04:00
Quick104andClaude Fable 5 1f9bd99990 fix(diagnostics): address round-4 review findings on PR #445
- schema: add crash/report.type conditionals (allOf if/then) so a
  crash/anr/native_crash/hang/abnormal_exit manifest requires `crash`
  and a `manual` manifest forbids it, matching ValidateManifest.
- service: reject uploads where X-Profile-Id and manifest.report.profile_id
  are both present but differ (new ErrProfileMismatch, mapped to 400
  profile_mismatch) instead of silently preferring the header; single-source
  and matching cases unchanged. Adds service tests for mismatch, match, and
  header-only attribution.
- schema: require manifest.json as the first archive.entries element via
  prefixItems (contains retained for validators without prefixItems support).
- schema: document that maxLength is a character-count bound while the server
  enforces UTF-8 byte length, via a top-level note and per-field notes on the
  free-text device_summary and crash fields.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
2026-07-21 12:58:28 -04:00
Quick104andClaude Fable 5 93b851fe81 fix(diagnostics): address round-3 review findings on PR #445
- Extend the upload write deadline alongside the read deadline so a slow
  upload finishing after the integrated server's 120s WriteTimeout can still
  return its success response instead of timing out a report that succeeded.
- Reject child-profile attribution for diagnostics: wire the attribution
  validator through a shared profile lookup that reports IsChild and drop
  attribution for child profiles, which must not perform diagnostics actions.
- Assert the download test captures the clicked anchor and checks its blob:
  href and silo-diagnostics-<short_id>.tar.gz filename, not just cleanup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
2026-07-21 12:25:11 -04:00
Quick104andClaude Fable 5 2d5d4980de fix(diagnostics): address round-2 review findings on PR #445
- settings.go: cap the parsed cleanup interval at 7 days before converting to
  time.Duration so a huge configured value can't overflow int64 nanoseconds and
  wrap into a tiny/negative interval; add boundary tests.
- settings.go: propagate genuine settings read failures from LoadSettings
  (missing/empty -> default, error -> fail) so a transient DB error surfaces
  retryably instead of silently reporting uploads disabled or wrong quotas.
- bundle.go: validate non-manifest bundle entries while streaming with bounded
  memory -- device.json and crash/*.json must be a single JSON object,
  logs.jsonl/breadcrumbs.jsonl must be newline-delimited JSON objects with a
  per-line byte cap (new contract.MaxLogLineBytes); binary members stay opaque.
- diagnostics upload handler: extend the read deadline per-route via
  http.ResponseController.SetReadDeadline (10m) so slow mobile uploads of large
  bundles aren't cut off by the shared 30s server ReadTimeout.
- web admin download: request the ?proxy=1 streaming path directly so downloads
  work when S3Private is only server-reachable and errors can surface in-page.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
2026-07-21 11:29:02 -04:00
Quick104andClaude Fable 5 4fa84a661a feat(diagnostics): client diagnostics server foundation
Implements slice 1 of docs/design/2026-07-19-client-diagnostics.md: the
versioned contract (schemas, fixtures, Go validator), storage-validated
diagnostics.uploads_enabled gate, account-scoped status endpoint, hardened
streaming multipart ingest with quota reservation and a receiving/ready/
failed report state machine, S3 streaming puts, acting-admin report API
(list/detail/download/delete with audit events), and the retention +
orphan-reconciliation cleanup task.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XppCCycoaskCsW7ja1fZct
2026-07-20 11:13:52 -04:00