Accept duplicate replay uploads without a read-back - #833
Open
Neonforge98 wants to merge 4 commits into
Open
Conversation
Pre-commit hook ran. Total eslint: 5, total circular: 0
Pre-commit hook ran. Total eslint: 5, total circular: 0
Pre-commit hook ran. Total eslint: 5, total circular: 0
Pre-commit hook ran. Total eslint: 5, total circular: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The cloud sync engine can get stuck re-uploading the same replay segments forever. Replay objects are content-addressed (the segment hash is part of the object key) and the storage policy grants INSERT but never UPDATE, so resuming an interrupted push re-uploads existing names and gets a duplicate rejection — Supabase reports it as HTTP 409, or as a 400 envelope carrying
{"statusCode":"409","error":"Duplicate","code":"KeyAlreadyExists"}.The old code refused to trust the rejection and confirmed existence with a HEAD read-back first. But member reads walk the session read ladder, which denies replay reads on metadata-only shares — for those sessions the probe can never succeed, so a segment that is already stored was treated as a failed upload on every engine pass, forever. In a long-lived workspace this produced a permanent per-pass error loop (
cloud push failed for session …: replay object upload failed with 400 … KeyAlreadyExists) and the affected sessions' pushes never concluded.Fix
Split the rejection classification in
uploadReplayObject:Duplicate/KeyAlreadyExists/already exists) now counts as success outright — content addressing guarantees the stored bytes are the ones we were sending, and no read-back is required.Verification
KeyAlreadyExistswith an unreadable object) asserting success with no probe call.Retention parking (second commit)
Removing the duplicate false-failure exposed the loop's other layer: sessions past the org's retention window fail their push with
ORG2_RETENTION_EXPIRED, and the engine re-walked the full doomed upload chain (a dozen storage POSTs plus the rewrite RPCs) for each of them on every pass. Retention only recedes further within a signed-in run, so the retry can never succeed.The push loop now parks a session when its push fails with
ORG2_RETENTION_EXPIRED— one journal entry (session_retention_parked) plus a log line, then the session is skipped for the rest of the run. The parked set clears onresetSyncState()(sign-in cycle, endpoint switch, app restart), which is also the boundary where an entitlement upgrade would re-evaluate.Live-verified on the same workspace: each of the three expired sessions walks its final failing push exactly once and logs
cloud push parked for retention-expired session …; subsequent passes are silent — no repeated uploads, no repeated RPC failures. Unit tests cover both directions (parks onORG2_RETENTION_EXPIREDwith the journal entry; other codes keep retrying).Desktop notification action APIs (third commit)
Every desktop launch logged two errors from the Team Inbox notification wiring:
register_action_typesand the action listener are mobile-only in the notification plugin — its desktopinvoke_handlerregisters just notify/permission commands, so both calls can only fail with "Command not found". The action button they were meant to provide has never existed on desktop.The two service entry points are now inert seams (kept for a future mobile target) instead of doomed plugin calls. Plain notifications, sounds, and the dock badge are untouched; the only observable change is that the two boot errors are gone — verified live on a fresh launch, and the Settings test notification still sends successfully through the plugin path.
Boot-time 401 storm (fourth commit)
Every cold start with an aged JWT fired a burst of unauthorized RPCs —
get_cloud_capabilities(team-inbox and realtime probes) andcloud_list_team_inbox_mentionswent out with the persisted, possibly-expired token before anything refreshed it, contributing a steady stream of 401s to the backend error rate on every client launch.Both surfaces now resolve token freshness before their first request: the team-inbox mentions client routes every RPC through a
freshestTokenchoke point (refresh + CAS write-back via the existinggetFreshCloudAccessTokenidiom, falling back to the passed token outside a running store), and the realtime capability probe uses the sameensureFreshSession+commitRefreshedAuthdance its sibling entitlement refresher already used.Live-verified by force-expiring the persisted token and cold-starting: zero 401s in the boot console, and the org-session listings succeed on the silently refreshed token.
Together with the retention parking and duplicate-upload fixes above, this removes the three client-side error generators observed against the managed backend: the per-pass Postgres RPC failure storm from expired sessions, the storage duplicate-upload rejections, and the cold-start 401 burst.