* 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>
- service.go: reject trailing data after the decoded manifest object.
Decoder.More() only reports array/object iteration, so a stray closing
delimiter (e.g. {...}}) slipped through where json.Unmarshal used to
reject it. Require the stream to reach io.EOF after decoding on both
the received and embedded sides; add a regression test.
- repo.go: split the list projection from cleanup. reportListSelectSQL
keeps the app_build JSONB extraction for the admin list; new
reportCleanupSelectSQL omits it so retention/stale batches don't touch
each candidate's manifest JSONB just to delete a row.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
- 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
- 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
- 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
- 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
- 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
- bundle: reject tar entry names that differ from their trimmed form instead
of normalizing padded names into the allowlist
- repo: reserve expected bytes on receiving rows and count receiving+ready in
the per-user byte quota so concurrent/multi-node uploads can't overshoot
- contract: require the crash object for event report types and keep it absent
for manual; add contract tests
- settings/service: seed diagnostics.server_instance_id atomically via
insert-if-absent and adopt the winning value across nodes
- bundle/service: capture the embedded manifest.json during ValidateBundle and
reject reports whose embedded manifest disagrees with the part-1 manifest
(minus archive); add tests
- admin: delete the DB row before the blob on DeleteReport; log bucket/key when
the blob delete fails instead of leaving a visible report with a missing bundle
- bundle: reject PAX/GNU tar formats and extension records that smuggle bytes
past validation; add a PAX-archive rejection test
- migration: add CHECK constraints for state, report_type, and platform
- docs: add text/jsonc language identifiers to the two unfenced code blocks
- cleanup: log-and-continue per report and aggregate errors so one poisoned
report no longer blocks the whole run; update tests
- tasks: give diagnostics its own cleanup interval key instead of reusing the
opslog key, and bound the startup settings lookup
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
Two validator behaviors made the contract unimplementable for clients
using standard tar libraries:
- Any byte after the tar end-of-archive marker was rejected, but GNU
tar, Python tarfile, and Apache Commons Compress all pad the archive
with zero blocks to a record boundary. Accept up to 64 KiB of zero
padding; any non-zero trailing data is still rejected.
- uncompressed_bytes was computed as the sum of entry payloads, which
no tar-producing client observes. Define it as the total decompressed
tar stream (headers, end-of-archive marker, and padding included) —
the byte count between a client's tar writer and gzip writer, and
what gzip -l reports. Documented in the design doc and contract.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
pgx binds a nil Go slice as SQL NULL, which bypasses the column's '{}'
default and violates its NOT NULL constraint, so reports without
playback session ids failed to insert. Bind an empty slice instead.
The ingest path also swallowed the underlying insert and bundle
validation errors, logging only a generic rejection reason; both sites
now log the real error so failures are diagnosable from server logs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh
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
