refactor(edge,router): claim router work durably before provider invocation#51
Merged
Merged
Conversation
…cation 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
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, CAND-4 (PR 4 in the master plan): operator-router's idempotency only became durable after provider work ran. Two concurrent requests for the same idempotency key could both find nothing completed, both invoke the OpenAI-compatible provider, and the losing response was simply discarded after the winner's debit landed — wasted provider spend with no compensating record.
This durably claims a router attempt before any provider invocation, so at most one caller ever owns "go call the provider" for a given
(user_id, idempotency_key)at a time.What changed
supabase/migrations/20260722000000_router_attempt_claim.sql:router_attemptstable +router_attempt_claim/router_attempt_complete/router_attempt_failRPCs (service_role only). A claim that's neither completed nor explicitly failed is recoverable once its lease elapses. Additive only — doesn't touchcredit_ledgerordebit_credits.packages/edge-shared/src/index.ts:claimRouteAttempt,completeRouteAttempt,failRouteAttempt,findDebitedRouteResultown the RPC/route-metadata encode-decode boundary.createOperatorRouterHandlernow claims → invokes provider → completes → 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. Legacycredit_ledgerrows written before this migration remain authoritative for old completed routes, independent of whether arouter_attemptsrow exists.supabase/functions/operator-router/index.ts: wires the service-role client straight through; drops the adapter-ownedfindCompletedRoutequery anddebitcallback.packages/edge-shared/test/edge-shared.test.ts):FakeRouterServiceSupabasereimplements the claim/complete/fail/debit RPC contract, covering the full choreography, live claims (409 attempt_in_progress), lease recovery, and debit-retry-without-re-invoking-the-provider.supabase/tests/database/router_attempt_claim.test.sql): claim/in_progress/completed outcomes, idempotent completion replay, failure recovery, lease-based recovery, per-user isolation, privilege grants, input validation.packages/edge-shared/test/router-attempt.contract.test.ts, following test(billing): checkout lifecycle contract tests against local Postgres #47/refactor(sync): durable settings-sync LWW (#37 CAND-3) #50's convention): proves the durable functions against real Postgres, including two genuinely concurrent claims for the same key resolving to exactly one claim, and a fullcreateOperatorRouterHandlerrun proving the provider is invoked and the ledger is debited exactly once across two concurrent same-key requests. Wired intobun run test:supabase.@pickforge/*0.11.0 → 0.12.0 and re-pins every Deno function'snpm:import (existing publish-then-pin convention).Validation
bun run typecheck✅bun run test— 366/366 ✅bun run build✅supabase db reset— all migrations apply cleanly ✅bun run test:supabaseagainst real local Postgres:router_attempt_claimlane) ✅welcome-credits-concurrency.ts✅checkout-lifecycle.contract.test.ts— 20/20 (still green) ✅supabase db advisors --local --type security --fail-on warn— clean ✅deno checkagainst the real npm registry isn't possible pre-publish (same documented gap as refactor(edge,billing): move fence/register/expire RPCs next to the deep lifecycle #48: published@pickforge/*is still behind main). Confirmed the pre-existing gap is unchanged — this diff's remainingdeno checkerrors are a strict subset of main's — and validated the adapter's typing directly against localsrcviatscinstead.Closes the "Router work is durably claimed before provider invocation" checklist item in #37.