refactor(sync): durable settings-sync LWW (#37 CAND-3)#50
Merged
Conversation
Replace the settings-sync last-writer-wins choreography (conditional `.update()` guarded by `updated_at <`, falling back to an `ignoreDuplicates` `.upsert()`, falling back to a third read to learn the outcome) with one durable `settings_sync_lww_write` SQL function that locks the (user_id, field_group) row, compares/writes/tombstones, and always returns the authoritative row in the same call. pushGroup/deleteGroup keep all validation/sanitization and now just call the durable RPC; `written`/`stale` result semantics are unchanged. The migration is additive only (#35): it adds the function, nothing else. Adds a local-Postgres contract lane (packages/sync/test/lww.contract.test.ts) following PR #47's pattern (skips cleanly without Postgres, wired into `test:supabase`), covering real concurrent push/push, first-insert/ first-insert, and push/delete races, equal-timestamp ties, and older data racing in after a newer tombstone. pgTAP coverage (supabase/tests/database/settings_sync_lww.test.sql) adds microsecond canonicalization, the existing future-timestamp check constraint, and per-user isolation. Issue #37 CAND-3.
7 tasks
ElbertePlinio
added a commit
that referenced
this pull request
Jul 19, 2026
…cation (#51) Issue #37 (CAND-4): operator-router's idempotency only became durable AFTER provider work. The handler checked `credit_ledger` for a completed route, invoked the OpenAI-compatible provider, and only then wrote a debit row keyed by the idempotency key — two concurrent requests for the same key could both find nothing completed, both call the provider, and the losing response was simply discarded after the winner's debit landed (wasted provider spend with no compensating record). New `router_attempts` table and three service-role-only RPCs (`router_attempt_claim` / `router_attempt_complete` / `router_attempt_fail`, supabase/migrations/20260722000000_router_attempt_claim.sql) move the durable decision in front of the provider call: at most one caller ever owns "go call the provider" for a given (user_id, idempotency_key) at a time. A claim that is neither completed nor explicitly failed is recoverable once its lease elapses. This is additive only: existing `credit_ledger` rows written before this migration remain authoritative for old completed routes (`findDebitedRouteResult`), independent of whether a `router_attempts` row exists for that key. `createOperatorRouterHandler` now claims before invoking the provider, completes the claim with the provider outcome, then debits — releasing the claim immediately on a definitive provider failure, and skipping straight to a debit retry (no re-invocation) when a claim is found already completed. - `packages/edge-shared/src/index.ts`: `claimRouteAttempt`, `completeRouteAttempt`, `failRouteAttempt`, `findDebitedRouteResult` own the RPC/route-metadata encode-decode boundary; `debitCredits` is unchanged. `createOperatorRouterHandler` takes `serviceSupabase` instead of separate `findCompletedRoute`/`debit` callbacks, and returns 409 `attempt_in_progress` while another claim is live. - `supabase/functions/operator-router/index.ts`: wires the caller's service-role client straight through; drops the adapter-owned `findCompletedRoute` query and `debit` callback. - `packages/edge-shared/test/edge-shared.test.ts`: replaces the `findCompletedRoute`/`debit` fakes with a `FakeRouterServiceSupabase` reimplementing the claim/complete/fail/debit RPC contract, covering the full claim -> provider -> complete -> debit choreography, live claims, lease recovery, and debit-retry-without-re-invoking-the- provider. - `supabase/tests/database/router_attempt_claim.test.sql`: pgTAP coverage for claim/in_progress/completed outcomes, idempotent completion replay, failure recovery, lease-based recovery, per-user isolation, privilege grants, and input validation. - `packages/edge-shared/test/router-attempt.contract.test.ts` (+ `router-fixtures.ts`, `postgres-router-client.ts`): a local-Postgres contract lane, following #47/#50's convention, proving the durable claim/complete/fail/debit functions against real Postgres, including two genuinely concurrent claims for the same key resolving to exactly one claim, and a full `createOperatorRouterHandler` run proving the provider is invoked and the ledger is debited exactly once across two concurrent same-key requests. Wired into `bun run test:supabase`. - Bumps `@pickforge/*` 0.11.0 -> 0.12.0 and re-pins every Deno function's `npm:` import, per the existing publish-then-pin convention. Validated: `bun run typecheck`; `bun run test` (366/366); `bun run build`; `supabase db reset` (all migrations apply cleanly); `bun run test:supabase` against real local Postgres — pgTAP (178 tests, including the new router_attempt_claim lane), welcome-credits concurrency, the CAND-1 checkout-lifecycle contract lane (20/20) and CAND-3 sync LWW contract lane (8/8) stay green, and the new router attempt contract lane (8/8) all pass; `supabase db advisors --local --type security --fail-on warn` clean. `deno check` against the real npm registry isn't possible pre-publish (same documented gap as #48: published `@pickforge/*` is still behind main) — confirmed the pre-existing gap is unchanged (this diff's remaining `deno check` errors are a strict subset of main's) and validated the adapter's typing directly against local `src` via `tsc` instead.
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
1 participant
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.
Summary
Issue #37, PR 3 (durable sync last-writer-wins).
The settings-sync last-writer-wins decision used to be client-driven
choreography: a conditional
.update()guarded byupdated_at <, afallback
ignoreDuplicates.upsert()on a miss, and a third read tolearn which row won. That reconstructed a durable concurrency decision
out of retries instead of making it one durable operation.
public.settings_sync_lww_write(new forward migrationsupabase/migrations/20260720000000_settings_sync_lww.sql, additiveonly per chore: reconcile hosted Supabase migration history #35): locks the
(user_id, field_group)row, compares theincoming
updated_at, and performs whichever of insert/update/no-opwins, returning the now-authoritative row in the same call.
security invoker, so it stays inside the table's existing RLS boundary(
auth.uid() = user_id), same pattern aspublic.credit_balance_cents.pushGroup/deleteGroup(packages/sync/src/index.ts) keep allvalidation/sanitization and now just call the durable RPC.
written/staleresult semantics are unchanged; obsolete retry choreography isremoved.
(
packages/sync/test/lww.contract.test.ts+lww-fixtures.ts/postgres-sync-client.ts), following PR test(billing): checkout lifecycle contract tests against local Postgres #47'spattern: skips cleanly with a console message when no local Postgres
is reachable, wired into
test:supabase. Covers real concurrentpush/push, first-insert/first-insert, and push/delete races, equal-
timestamp ties (first writer wins), and older data racing in after a
newer tombstone (no resurrection).
supabase/tests/database/settings_sync_lww.test.sql)adds microsecond canonicalization, the existing future-timestamp
check constraint, and per-user isolation.
Validation
bun run typecheckbun run test— 353/353supabase db reset— all migrations apply cleanly (additive-onlyconfirmed)
PICKFORGE_ALLOW_LOCAL_DB_TESTS=1 bun run test:supabaseagainstlocal Postgres: pgTAP (155 tests incl. new lane), welcome-credits
concurrency, the CAND-1 checkout-lifecycle contract lane (20/20,
still green), and the new sync LWW contract lane (8/8) — all pass
supabase db advisors --local --type security --fail-on warn— noissues
Closes part of #37 (CAND-3).