fix(billing): fix-forward refus expert PR #27/#28 — durabilite org-scoped, serialisation, atomicite, validation org/devise, compensation derivee - #39
Conversation
…erialisation, atomicite, validation, compensation derivee Reponse expert 2026-07-21 §D. Les PR etant mergees, correction fix-forward. PR #27 (import safety billing) : - reservation DURABLE dans le grand livre #28 (DurableImportCreditLedger sur LedgerReservation) — survit au redemarrage ; l'in-memory ne sert que les tests sans DB - idempotence scoppee organisation : unique DB (organizationId, idempotencyKey) — plus de cle brute partageable entre orgs - creation SERIALISEE : la reservation (create/catch-P2002) est acquise AVANT la creation du job ; de 2 POST concurrents meme cle, exactement 1 cree le job, l'autre rejoue (poll puis 409 IMPORT_CREATE_IN_PROGRESS) - ownership : settle/get verifient l'organisation appelante (BILLING_RESERVATION_FOREIGN) PR #28 (grand livre double entree) : - UNE transaction DB pour l'etat ET les ecritures (postEntriesInTrx dans commit/release/compensate/reserve) — plus de COMMITTED sans settlement - hard limit SERIALISE : SELECT FOR UPDATE sur le compte reserved + balance/check/post dans la meme transaction - postTransaction valide chaque compte : appartenance organisation + devise (LEDGER_ACCOUNT_ORG_MISMATCH / LEDGER_ACCOUNT_CURRENCY_MISMATCH), rien de persiste au refus - compensation DERIVEE des ecritures de settlement persistees (deriveCompensationEntries) — taxMinor n'est plus refourni par l'appelant Preuves vrai Postgres : import-billing-db.spec.ts A1-A4 + B1-B4 (8/8) ; ledger-store-db.spec.ts 7/7 inchanges ; 96/96 sur 10 suites ; preuve route concurrente 2 POST -> 1 seul ImportJob. Contrats BILLING_LEDGER / IMPORT_REMIX v3 (ancres a jour : PR mergees + PR de correction). Evidence : docs/deploy-evidence/2026-07-21-billing-fix-forward/. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96f53af716
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| reservedCredits: estimateImportReservation(stagedFiles.length), | ||
| }); | ||
|
|
||
| if (!held.created) { |
There was a problem hiding this comment.
If the process dies or createImportJob fails after reserve() but before attachJob(), the durable row permanently has no importJobId. The expiry sweep only changes that row to EXPIRED; it does not remove it, while future reserveUsage calls still return it with created: false. Every retry with the required idempotency key therefore enters this branch, polls an attachment that can never appear, and returns IMPORT_CREATE_IN_PROGRESS forever. Expired/released unattached reservations need a reclaim path, or reservation and job creation/attachment must be atomic.
Useful? React with 👍 / 👎.
| * enforces "committed === true" and the no-debit-without-commit invariant. | ||
| */ | ||
| const settled = importLedger.settleByJob(importJobId, true, estimateImportReservation(finalFiles.length)); | ||
| const settled = await importLedger.settleByJob(orgId, importJobId, true, estimateImportReservation(finalFiles.length)); |
There was a problem hiding this comment.
This newly fallible database settlement runs only after the project row, files, manifest, usage record, and COMMITTED job state have already been persisted. If the ledger transaction fails—for example during a transient database error—the catch path only calls cleanupImport; it never deletes the created project or its files, and it may release the still-active hold with zero debit. The request consequently reports failure/rollback while leaving a usable unbilled target behind, so target creation and settlement need recovery or explicit target compensation.
Useful? React with 👍 / 👎.
| const projected = -net + input.maxAmountMinor; | ||
| assertWithinHardLimit(projected, input.hardLimitMinor, 'reservation budget'); |
There was a problem hiding this comment.
When two identical reserveUsage calls with a hard limit race, both can miss the initial reservation lookup. After the first commits, the second acquires this lock, counts the first call's hold as additional usage, and can throw LEDGER_HARD_LIMIT here before reaching the unique reservation insert and its P2002 replay handling—for example, two retries of a 70-unit reservation under a 100-unit cap. A concurrent idempotent retry is thus rejected even though it creates no additional hold; recheck the (organizationId, idempotencyKey) row after acquiring the lock.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Fix-forward for the expert refusals on PR #27/#28 by making import credit reservations durable and org-scoped, serializing concurrent idempotent creates, and ensuring ledger reservation lifecycle operations are atomic and validated in Postgres-backed flows.
Changes:
- Add a durable import reservation backend (
DurableImportCreditLedger) wired into the API when using the Prisma store; update create/replay logic to use the durable reservation as the serialization lock. - Refactor
LedgerStorereservation lifecycle methods to post state transitions + ledger entries inside a single DB transaction; add account org/currency validation and derived compensation from persisted settle entries. - Add/extend real-Postgres proof tests for org-scoped idempotency, serialization, ownership, atomicity, hard-limit concurrency, and derived compensation; update route tests for concurrent create behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| services/api/src/tests/import-routes.spec.ts | Adds concurrent idempotent-create route proof cases; minor formatting changes. |
| services/api/src/tests/import-billing-db.spec.ts | New DB-backed proof suite (A1–A4, B1–B4) against real Postgres for the fix-forward requirements. |
| services/api/src/ledger-store.ts | Introduces postEntriesInTrx, makes reservation lifecycle operations atomic, validates account org/currency, serializes hard-limit checks, and derives compensation from persisted settle entries. |
| services/api/src/ledger-reservation.ts | Adds deriveCompensationEntries helper to compute reversal entries from persisted settle legs. |
| services/api/src/import-billing.ts | Defines shared ImportBillingLedger interface and upgrades in-memory ledger to org-scoped keys + ownership-checked by-job operations. |
| services/api/src/import-billing.spec.ts | Updates tests for the new async/org-scoped in-memory ledger behavior and expert refusal cases. |
| services/api/src/import-billing-durable.ts | New durable import billing ledger implementation backed by LedgerStore/LedgerReservation in Postgres. |
| services/api/src/app.ts | Wires durable ledger when Prisma-backed; uses durable reservation for serialized idempotent create and adds durable reap path. |
| docs/parity/IMPORT_REMIX_CONTRACT.md | Updates contract anchor/history to v3 and documents fix-forward evidence. |
| docs/parity/BILLING_LEDGER_CONTRACT.md | Updates contract anchor/history to v3 and documents fix-forward evidence for ledger changes. |
| docs/deploy-evidence/2026-07-21-billing-fix-forward/README.md | Adds a consolidated fix-forward evidence README and replay instructions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* | ||
| * The durable reservation is the lock: exactly ONE request creates (201); | ||
| * the other replays the winner's import (200) or is told to retry (409) — | ||
| * it NEVER creates a second job. | ||
| */ | ||
| expect(codes[0]).toBe(200); | ||
| expect(codes[1]).toBe(201); | ||
|
|
||
| const jobs = [...store.importJobs.values()].filter((job) => job.organizationId === org.id); | ||
| expect(jobs).toHaveLength(1); | ||
|
|
||
| const replay = first.statusCode === 200 ? first.json() : second.json(); | ||
| const fresh = first.statusCode === 201 ? first.json() : second.json(); | ||
| expect(replay.import.replayed).toBe(true); | ||
| expect(replay.import.importJobId).toBe(fresh.import.importJobId); | ||
| }); |
| // Bind the reservation (already held, pre-paid-work) to the created job. | ||
| await importLedger.attachJob(orgId, body.idempotencyKey, job.id); | ||
| await store.updateImportJob(job.id, { creditsReserved: true }); |
| // Derive the reversal from what the settle ACTUALLY posted. | ||
| const persistedSettle = reservation.settleTxId | ||
| ? await this.db.ledgerEntry.findMany({ | ||
| where: { transactionId: reservation.settleTxId }, |
| export interface ImportBillingLedger { | ||
| reserve(input: { | ||
| organizationId: string; | ||
| key: string; | ||
| reservedCredits: number; | ||
| }): Promise<{ reservation: ImportReservation; created: boolean }>; |
… total), 7 refus inscrits, décision Identity.kind, attestation durcie (head_branch + test déterministe) Reçu -05 COMPLET (réponse brute archivée incoming/, sha256 c32a945d…, commit audité 60a987c) : V3-02/A2-02/A2-11/LS-05/LS-17 CLOSED avec limites de portée verbatim. État dérivé : 24 CLOSED / 6 PRP / 30 OPEN / 5 PROVEN = 65. Refus inscrits verbatim : V4-1 (SCINDÉ — le point ne porte plus que « routes produit rendues JS », canal de lancement → UNK-LAUNCH-CHANNEL), V4-2 (mesure alignée : « 43 ancres /gallery/ dans le DOM », plus de « cartes »), LS-13 (HAR/trace Playwright d'un run unique — session capture dédiée), LS-16/LS-18/ V3-14 (attestation), A2-09 (5 défauts repro WIF — session PR #38) ; CTR- OPERATIONS-DR + 2 lots PR #39 consignés au reçu. Attestation durcie (RR-05 §1-2, rejoué) : verify-attestation-run vérifie run.head_branch=='main' ; test de substitution DÉTERMINISTE par fixture figée (capture API réelle du run étranger 29812663423) + contre-contrôle head_branch seul + sonde live best-effort non bloquante. Positif re-authentifié. DEC-IDENTITY-KIND enregistrée (USER/GUEST/SERVICE, décision Avi) et ancrée dans IDENTITY_COLLABORATION_CONTRACT (ancienne énumération remplacée, tracée). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lines, compensation de la cible, retry idempotent sous plafond, checks CI Defaut A (#39-1): LedgerStore.reviveReservation — un hold MORT jamais attache (EXPIRED/RELEASED: re-arme + hold re-poste en UNE transaction ; ACTIVE perime: extension seule, sans doubler le hold) est recupere par le retry de la MEME cle, qui procede en createur. Un hold VIVANT non attache n'est jamais re-arme (pas de fork de job). Fini l'IMPORT_CREATE_IN_PROGRESS eternel. Defaut B (#39-2): le settlement passe AVANT le tampon COMMITTED ; sur echec de settlement la cible est explicitement compensee (hardDeleteProject — quota projects.count restitue par le count vivant), job ROLLING_BACK, hold relache. Plus jamais de cible utilisable non facturee. Defaut C (#39-3): reserveUsage re-verifie (organizationId, idempotencyKey) immediatement APRES le verrou FOR UPDATE et AVANT le calcul du plafond — un retry idempotent concurrent REJOUE au lieu d'etre refuse LEDGER_HARD_LIMIT. Preuves vrai Postgres (12/12 import-billing-db.spec.ts): C1 (2 retries concurrents meme cle sous limite 100/hold 70 -> zero refus, 1 reservation), A1 (cle expiree -> retry created, attach+settle, compte reserved a zero), A2 (stale-ACTIVE -> extension, 1 seule ecriture de hold), A3 (hold vivant -> replay, pas de revive). Preuves route (13/13): settle en echec -> 0 projet survivant + ROLLING_BACK + hold relache ; cle morte -> retry 201, 1 job. 102/102 sur 10 suites, log BRUT committe en .txt (test-runs-raw.txt — le .log de la ronde 1 etait avale par *.log du .gitignore, d'ou le 404 constate). Checks CI: fix admin-model.test (32 sections, defaut preexistant de main, meme correctif que PR #40 non mergee) -> Install/test/build/scan et Quality Gates ; DOCUMENT_MANIFEST.yaml regenere -> Validate registries. Production E2E/Playwright: 51 tests ui-details-* rouges sur TOUTES les branches (runs 29893345877/29838622737/29838616226, PR docs-only incluses) — chantier repo-wide hors perimetre. Contrats v4 (refus v3 trace motif par motif). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ronde 2 — réponse au second refus (commit 700ce71)Défaut A (#39-1) — réservation orpheline sans importJobId : Défaut B (#39-2) — cible utilisable non facturée : settlement AVANT le tampon COMMITTED ; sur échec, Défaut C (#39-3) — retry idempotent refusé sous plafond : re-vérification de (organizationId, idempotencyKey) immédiatement APRÈS le verrou FOR UPDATE, AVANT le calcul du plafond. Preuve PG C1 : 2 retries concurrents même clé, limite 100/hold 70 → zéro LEDGER_HARD_LIMIT, 1 réservation. Logs bruts : Checks : fix PROVEN_REVIEW_PENDING — pas de merge sans feu vert. |
…aire de values-prod.yaml contenant 'fake'
Defaut preexistant de main (commentaire du lot D2 rollback, merge) expose par
cette PR: premiere a depasser le test admin-model, elle atteint l'etape
Security checks qui greppe \bfake\b y compris dans les commentaires.
Reformulation pure ('URL-copy pseudo-rollback'), zero changement de valeurs
Helm. Garde rejoue en local: 'runtime mock scan clean' exit 0 +
production:validate:self-test exit 0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
État final des checks (commit 40bc92b)16 pass / 4 skipping (conditionnels normaux) / 1 fail. Verts notamment : Install, test, build, scan · Quality Gates · Validate registries · CodeQL ×2 · gitleaks · Secrets Detection · Dependency Scan · Accessibility · Performance · Quality Analysis · Release Validation. Chemin parcouru (3 causes séquentielles, toutes préexistantes de main, exposées par cette PR) :
Seul rouge restant : « Playwright local stack » — 51 tests PROVEN_REVIEW_PENDING — pas de merge sans feu vert. |
…l-closed)
Refus du correctif Import/billing : une course subsistait entre récupération
d'une réservation orpheline, reaper d'expiration et attache du job — le reaper
pouvait expirer une réservation qu'un revive venait de ré-armer, puis le job
s'attachait à une réservation EXPIRED (prédicat sans status ACTIVE).
- Colonne version (optimistic lock, migration 0079) bumpée à CHAQUE transition
(reserve/revive/release/expire/commit/compensate) — sérialise revive/reap/attach.
- reapExpiredReservations sélectionne {id, version} et l'épingle : le CAS
releaseReservation(timeout) exige expiresAt<=now ET version=sélectionnée →
une revive (expiry étendue + version bumpée) fait matcher 0 ligne, le reaper
n'expire plus un hold ravivé.
- attachJob(org,key,job,expectedVersion) : CAS where {importJobId:null,
status:ACTIVE, version:expectedVersion, expiresAt>now} — refuse d'attacher à
une réservation expirée ou de mauvaise génération ; la route abandonne
fail-closed (IMPORT_RESERVATION_EXPIRED_BEFORE_ATTACH).
Preuves vrai Postgres (74/74 sur 6 suites) : C1 interleaving EXACT (reaper
sélectionne → revive étend+bumpe → CAS reaper échoue → reste ACTIVE → attach
à v+1), C2 fail-closed (expirée → attach = conflict, importJobId null), C3
fuzz concurrent 12× (jamais d'EXPIRED avec un job attaché).
Evidence: docs/deploy-evidence/2026-07-23-reservation-race/.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ronde 3 — course reaper/revive/attach fermée (commit 22dbfaa, expert V3 §C)L'interleaving refusé est clos fail-closed :
Preuves vrai Postgres (74/74, 6 suites) —
PROVEN_REVIEW_PENDING — pas de merge sans feu vert. |
…ED (2 signés), 4 refusés + 2 contrats refusés Reçu -07 COMPLET (réponse brute incoming/, sha256 f5771529…, commits audités §1, décision machine §9). SIGNED/CLOSED reviewer OpenAI-Codex UNIQUEMENT : - P0-LS-13 (session navigateur fail-closed, observations reliées au run) - P0-LS-03 (couverture de hash du paquet, job CI officiel) avec limites de portée verbatim. Rien d'autre. REFUSED (motifs verbatim) : P0-LS-16/LS-18/V3-14 — le vérificateur v6 reste FAIL-OPEN sur l'ABSENCE de mergedCommit/repoCommit/runUrl (contrôlés seulement s'ils existent) ; correction v7 exigée (champs obligatoires non vides + négatifs par suppression + roll post-merge). P0-A2-09 — repro.sh pose le trap teardown APRÈS create+billing (fuite projet possible). Contrats REFUSED_V4, reviewer NON écrit : CTR-OPERATIONS-DR (obligations BLOCKED/UNTESTED ; sous-artefacts = preuves individuelles) + CTR-RUNTIME-NIX (négatif live non exécuté). PR #39 ACCEPTÉE À PORTÉE CIBLÉE — AUCUNE signature de CTR-BILLING-LEDGER ni preuve de prod déduite. PR #51/#52 refusées. Compteurs : 30 CLOSED / 27 OPEN / 5 PROVEN / 3 PROVEN_REVIEW_PENDING = 65. IMPLEMENTATION_STATUS + vues de suivi régénérées, validateur vert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix-forward — refus expert des PR #27 (import billing) et #28 (grand livre) — 2026-07-21 §D
Les deux PR sont mergées sur main → correction fix-forward, pas de revert. Statut : PROVEN_REVIEW_PENDING — pas de merge sans le feu vert d'Avi, pas de clôture avant reçu de revue.
Défauts corrigés 1 à 1
PR #27 :
LedgerReservation: unique DB(organizationId, idempotencyKey). Preuve PG A1 : même clé, 2 orgs → 2 réservations indépendantes.created: truecrée le job. Preuves : PG A2 (2 reserves concurrentes, 2 connexions → exactement 1 created) + route (2 POST simultanés → 1 seul ImportJob, rejeu même jobId).DurableImportCreditLedger). Preuve PG A3 : réservée par le client A (déconnecté), lue et settlée par le client B.settleByJob/getByJobvérifient l'org appelante (BILLING_RESERVATION_FOREIGN). Preuve PG A4.PR #28 :
postEntriesInTrxdans une seule$transaction(commit/release/compensate/reserve). Preuve PG B1 : double-commit concurrent → 1 seul settlement ; 0 réservation COMMITTED sanssettleTxId.SELECT … FOR UPDATEsur le comptereserved, balance+check+post dans la même transaction. Preuve PG B2 : limite 100, 2×70 concurrents → exactement 1 accepté, 1LEDGER_HARD_LIMIT.deriveCompensationEntriesdérive la ventilation des écritures de settlement persistées ;compensateReservation(id)sans argument fiscal. Preuve PG B4 : settle 60 (tax 9) → reversal dérivé 51/9/60,reversalOfId= settleTx, tous les comptes à zéro.Chiffres
docs/deploy-evidence/2026-07-21-billing-fix-forward/test-runs.log), dont 8/8 nouvelles preuves contre vrai Postgres et 7/7 preuves PG existantes inchangées.0078(migrate deployOK).Contrats
BILLING_LEDGER_CONTRACT.md/IMPORT_REMIX_CONTRACT.md→ v3 : ancres corrigées (PR mergées + cette PR de correction), historique des refus v2 tracé, PENDING_REVIEW, rien d'auto-clôturé.Notes
LedgerReservationde 0078). Aucun changement de comportement HTTP hors le rejeu concurrent (409IMPORT_CREATE_IN_PROGRESSdans la fenêtre d'attache, ~<1 s).npx tsclocal résout TS 7.0.2 →TS5112sur le scriptbuild: préexistant sur main non modifié (environnement local), le CI utilise TS 5.x du lockfile.🤖 Generated with Claude Code