feat(sync): execute provider update commands with safe replay#2255
Merged
Conversation
Route an update of a single, provider-linked event to the owning provider when provider work is active. The hard part is replay safety: a successful conditional patch changes the provider version, so a naive crash-then-retry would re-send the now-stale expected version and the provider would reject it as a conflict — misreporting an edit that actually landed. So the executor fetches the provider's current state first. If it already carries the command's intended content, the edit landed on a prior attempt and the command is confirmed at the current version with no second write. Otherwise it patches conditionally (If-Match on the expected version), which turns a genuine concurrent external edit into a typed versionConflict failure. The content check only gates the replay shortcut, so a false miss falls through to the conditional patch — a spurious conflict at worst, never a lost external edit. The new content and version are committed to the canonical record (owner-scoped, non-upsert) before the command is confirmed. Adds the client-driven invitation to the update command (default none), passed through to the patch. Series events, single-to-series conversions, and provider move/delete remain deferred to later slices. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
matchesIntendedEdit deep-compared all of the event content, but the provider adapter only writes title/description/location/schedule — organizer, attendees, and conference are read-reflected and drift independently (an attendee RSVPs, etc.). So after a patch landed, a routine attendee response made the fetched content differ from the client's snapshot, the replay check missed, and the retry fell through to a patch conditioned on the now-stale version — turning an edit that already succeeded into a spurious versionConflict failure. Compare only the fields the patch actually writes. Add a regression test where the written fields match but an attendee has RSVP'd, asserting the replay is still recognized with no second write. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
What
The provider-update half of S28: routes an
updateof a single, provider-linked event to the owning provider (Google) when provider work is active, with replay-safe conflict handling — the "one safe replay" the ledger calls for. Cloud updates (S28.1), and series/move/delete, are unchanged/deferred.The replay-safety problem
Unlike provider create (idempotent for free via a deterministic id + Google's 409 read-back), a successful
patchEventchanges the version. So a crash-before-confirm retry would re-send the now-stale expected version → Google returns 412 → a naive executor reportsversionConflictfor an edit that actually landed. That's a false failure on a successful write.The design: fetch-first
executeProviderUpdate:fetchEventthe provider's current state.failed{permanentProviderError}(nothing to update).patchEventconditionally (If-Match= the command's expected version). A genuine concurrent external edit → 412 →failed{versionConflict}; transient → pending.replaceExisting(no resurrection of a concurrently-deleted event), then confirm.The content comparison only gates the replay shortcut, so a false miss fails safe: it falls through to the conditional patch, whose
If-Matchyields a spurious conflict at worst — never a silently-lost external edit.Invitation
Per the decision, the
updatecommand carries the client-driveninvitation(defaultnone), passed straight topatchEvent.Tests
If-Match+ invitation pass-through), replay (fetched content matches intent → confirm, zero patch calls), genuine conflict →failed{versionConflict}, provider-event-gone →failed{permanentProviderError}, transient patch → pending, revoked credential → failed with no provider call.invitationtonone; accepts an explicit one.All core (490) + sync (364) suites green.
🤖 Generated with Claude Code