fix(cloudsync): tell the failure modes apart, retry the ones worth retrying

[vishk23]

Stacked on #7 (fix/cloudsync-gate-leak) — based on its head, so this PR shows only its own commit. GitHub will retarget it to main when #7 merges.

The part of the ten days this fixes

#7 explains why a phone stopped syncing at all: the process-wide gate was stuck held, so every entry point bailed instantly. That is the whole story of why nothing ran. This PR is about the other half — what happened on the days something did run, and why nobody could tell.

The server was only broken for part of that window, and it was loudly specific about how. POST /ingest answers 507 insufficient_space when the volume is full, and its handler says in a comment exactly why the status matters: so the phone reads it as "server has no room, keep the backup and retry later" rather than "this upload is malformed, discard it". It answers 400 corrupt_sqlite for the opposite case. POST /deepbuf answers 503 storage_not_configured when object storage was never set up — an optional feature, not a fault.

The client threw all of that away:

enum CloudSyncError { case badResponse(Int, String); case network(String); case decode }
// "The cloud sync server returned an error (\(code))."

So every one of those became the same sentence with a different number in it. On the Data Sources card, side by side, sat:

The cloud sync server returned an error (507)
Deep buffers: The cloud sync server returned an error (503)

One of those was a real outage. The other was a feature that had never been switched on. Nothing in the app distinguished them, so one broken thing read as two, and the message that mattered was buried next to one that never did.

And there was no retry and no backoff anywhere. A single 500 from a restarting server ended the sync. The next attempt was not seconds later — it was whenever the 20h autoSyncIfDue or 4h backgroundSyncIfDue window next opened, and autoSyncIfDue runs once per launch on a process that stays resident. One unlucky second cost a day.

What changed

1. CloudSyncError becomes a taxonomy (Strand/CloudSync/CloudSyncError.swift). One place — CloudSyncClient.throwIfNotOK — turns (status, body) into exactly one case, driven by the server's own machine-readable error code, so no endpoint can grow a private interpretation of a status and no new endpoint can re-collapse them.

Server says	Case	Retry?
507 insufficient_space / storage_unavailable	serverOutOfSpace	yes
500/502/504, and any other 503	serverFault	yes
transport / timeout	network	yes
400 corrupt_sqlite, bad_zip, 413 too_large, 404	rejected(status:code:detail:)	no
401/403	unauthorized	no
503 storage_not_configured	featureNotConfigured	not an error at all
cancelled (CancellationError or URLError.cancelled)	cancelled	no
2xx that won't parse	decode	no

2. Bounded retry with exponential backoff + a jitter band (CloudSyncRetry.swift), consulting isRetryable and nothing else.

The budget is deliberately small: ~1s then ~3s, three attempts, ~4s worst case. The target cadence is a sync every time the phone is unlocked, so giving up early now costs one more unlock rather than one more day — which makes a long in-sync ladder the wrong trade. A retry here rides out a blip a human wouldn't notice; it does not try to out-wait an outage. CloudSyncClient.syncSession's 1h request ceiling and CloudSyncGate.staleHoldS's 2h reclaim both sit far above it, so the ladder can never itself be what wedges a sync.

/ingest gets its own tighter budget — one retry, not two. Resending a 100-300MB .noopbak is minutes of cellular data. It is still worth one, because the most common /ingest refusal (507) is answered from the declared Content-Length before the server accepts a byte of the body, so that attempt cost nothing to make and nothing to repeat.

Jitter is a full band around each delay rather than a bare doubling: every phone an outage knocked offline otherwise retries on the same schedule and returns as a thundering herd.

3. The deep-buffer 503 is a skip, not a failure. DeepBufferDrainSummary gains skippedNotConfigured, deliberately not error — error is what becomes a user-visible · Deep buffers: … suffix. The watermark stays exactly where it was, so the moment a bucket is configured the same bytes ship; nothing is written off. The lane is then not re-probed for 6h, because the 503 only arrives after the chunk body is sent, and re-uploading megabytes on every unlock to be told "no" is not a no-op. Six hours, not sticky, so provisioning heals on its own the same day. (Tigris has since been provisioned, so this path should now succeed — the handling still has to be right, and a 500 deepbuf_failed still reports as a real failure.)

4. cloudsync.lastStatus says something a human can act on. Every message now ends with what happens next:

• Failed — The server has no room for the backup right now (insufficient_space). Nothing was lost — your data is safe on this device and the next sync will try again. only 300 MB free · 4:12 PM
• Failed — Cloud sync rejected this device's token (401). Re-enter the server token in Data Sources — retrying won't help. · 4:12 PM

A failure is now prefixed, not merely worded differently: success and failure used to produce the same shape of sentence with the same trailing timestamp, so …returned an error (507) · 4:12 PM was easy to read at a glance as "it synced at 4:12" — and during the outage this string and its timestamp were the only signals available.

5. A pull that applied edits now uploads unconditionally. Found while tracing the same "a real change never reached the server" theme, and it is a genuine hole, not a hypothetical:

• contentToken() keys workout on COUNT alone. fix_workout tombstones the original, upserts the corrected copy under the noop-cloud device, then deletes the original — +1 then −1 — and touches no other table the token reads. Token byte-identical.
• It keys sleepSession on (COUNT, MAX(endTs)). adjust_sleep_bounds / edit_sleep_stages UPDATE a row in place, so restaging any night that is not the most recent moves neither. Token byte-identical.

In both cases the skip-unchanged gate silently declined to upload a correction the user had just made. ContentToken.swift's doc comment actively claimed the opposite ("still moves the hrSample/dailyMetric segments in the same edit"); that is false, and it now records the residue instead.

Fixed at the call site, not the fingerprint: summary.applied > 0 || didRecompute is the same fact, already computed, right where the decision is made. Redesigning the token would mean content-hashing tables in an upstream-shared package and would force every device one full re-upload just to change the token's format. The token keeps doing what it is good at — cheaply proving a device with no new samples and no applied edits has nothing to ship.

Deconfliction

• fix(cloudsync): stop a stopped sync from wedging every sync after it #7 (fix/cloudsync-gate-leak) — based on its head; zero overlap. fix(cloudsync): stop a stopped sync from wedging every sync after it #7 owns the gate lifetime and the session timeouts, this owns the error taxonomy and the retry ladder. The two bounds are complementary and the comments cross-reference: fix(cloudsync): stop a stopped sync from wedging every sync after it #7's staleHoldS (2h) > syncSession's resource ceiling (1h) > this PR's whole retry ladder (~4s).
• cloudsync-background-upload — that branch adds a fileUploader injection hook at the top of sendUpload(_:fromFile:) and makes IngestResponse internal; this PR changes the catch at the bottom of the same functions and the bodies of the public methods. git merge-tree reports the same two conflicting files before and after this commit (CloudSyncClient.swift, CloudSyncModel.swift) — those conflicts already exist between fix(cloudsync): stop a stopped sync from wedging every sync after it #7 and that branch, and this adds none.
• The BGTask identifier mismatch (CloudSyncBackgroundRefresh.bgTaskIdentifier / ScheduledDebugExport.bgTaskIdentifier hardcoding com.noopapp.*) is deliberately untouched here — a separate change owns it.

Verification

Strand/CloudSync/ is app-target Swift, so no default CI covers it — local builds are the verification.

Check	Result
xcodebuild Strand macOS, CLOUD_SYNC set	BUILD SUCCEEDED
xcodebuild Strand macOS, flag absent (default offline build)	BUILD SUCCEEDED
xcodebuild NOOPiOS, generic/platform=iOS	BUILD SUCCEEDED
StrandTests full suite	1071 tests, 1 skipped, 0 failures (1039 on #7 → +32)
swift test in Packages/WhoopStore	290 tests, 0 failures

The flag-absent build is checked deliberately: all of this stays inside #if CLOUD_SYNC, so "fully offline" remains a property of the default binary's bytes. The only change outside the gate is a doc comment in ContentToken.swift.

New tests worth naming:

• testEachServerStatusMapsToItsOwnTypedError — the whole table, one row per code
• test503IsOnlyNotConfiguredWhenTheServerSaysSo — the body, not the status, decides
• testOnlyTransientClassesAreRetryable — the classification the retry engine reads, in one place
• testUploadCorruptSqliteIsTerminalAndNeverResendsTheBody — exactly one request
• testUpload507SurfacesAsOutOfSpaceAndIsRetried — exactly two, never three
• testCancellationIsNeverMistakenForANetworkFault — both shapes land on .cancelled, so a BGAppRefreshTask at expiry never sits out a backoff it cannot survive
• testACancelledBackoffAbandonsTheRetryLoop — same hazard, from inside the ladder
• testNotConfiguredServerIsRecordedAsASkipNotAnError / …LeavesTheWatermarkUntouched / testARealDeepBufferFailureIsStillReportedAsAnError — the skip is scoped to one status, not an amnesty
• testTheWholeStandardLadderIsOverInSecondsNotMinutes — pins the budget against future creep
• testAFailureIsMarkedAsOneAndASuccessIsNot — the card cannot show a failure as if it synced

CloudSyncRetryPolicy.delayS(beforeAttempt:jitterUnit:) takes the random draw as a parameter and withCloudSyncRetry takes its sleep, so the entire schedule is pure arithmetic — nothing here sleeps and nothing is flaky. Same reasoning as #7's begin(now:).

Cross-platform

No Android twin — Android has no cloud sync at all. The only CloudSync hits under android/ are a Material Icons.Filled.CloudSync on the local backup screen. ContentToken.swift is in an upstream-shared package, but it has no Kotlin counterpart (grep -rn contentToken android/ → nothing) because it exists solely to serve this fork-only skip-unchanged gate — and the change there is a doc comment, altering no stored bytes and no computed value.