fix(cloud): settle swept app-chat holds markup-inclusively — sweep refunded the creator markup to the org (#11592 residual) - #11686
Conversation
The stranded-reservation sweep (#11493) settles a stale hold to metadata.estimated_cost. For app_chat_reservation_v1 holds that number is the UNBUFFERED BASE cost while the row amount is the org charge (buffered base + creator markup, computeInferenceCharge) — a unit mismatch: sweeping a stranded monetized app-chat hold refunded the org the creator markup too, while the creator's earnings recorded at deduct time stay put, so the platform ate the full markup on every swept monetized hold. Scale the org charge by estimated/reserved instead, so the org nets exactly what a normal settle at the estimated cost charges (base x (1 + markup)). Non-monetized holds are unchanged (markup 0 collapses to the old math). Holds without a usable base pair settle exact-cost; result is clamped to the held amount so corrupt metadata can never become an overage charge. Real-PGlite tests: monetized-shape hold swept exactly once (refund 0.60 on a 1.80 hold with 1.00 base estimate + 20% markup; re-sweep scans 0; late settle no-op) + missing-base-pair exact-cost noop. Refs #11592
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Money-path — needs [cloud-money]/@lalalune review. This fixes a real residual over-refund in the #11493 app-chat sweeper (which correctly closed #11592's core gap): swept monetized holds settled to the unbuffered base estimated_cost while the row amount is the org charge (buffered base + creator markup), so a stranded monetized hold over-refunded the org by the markup. Proven: RED pre-fix (org +$0.80 vs expected +$0.60), GREEN post-fix ($0.60 exactly once — single recon::refund row, re-sweep scans 0, late settle no-op); credits-reconcile 30/0, the 4 #11493 suites 68/0. Authored via Fable-5 ultracode. — |
|
|
…nblock ALL deploys (#11847) Every develop deploy since ~03:46 UTC failed at the Deploy API Worker step: No matching export in "src/stubs/elizaos-core.ts" for import "runWithTrajectoryPurpose" (shared/src/email-classification/email-classifier.ts:20) email-classifier (@elizaos/shared, pulled into the Worker bundle transitively) imports runWithTrajectoryPurpose from @elizaos/core, which the Worker aliases to this stub — but the stub only had runWithTrajectoryContext, not the newer runWithTrajectoryPurpose (added to core in trajectory-context.ts). So the esbuild bundle failed and NO deploy could ship — blocking the develop→main promote that carries the merged money fixes (#11810 withdraw, #11817 book-influencer, #11686 sweep) to prod. Fix: add the Worker-safe stub mirroring runWithTrajectoryContext — no trajectory context manager exists in the Worker bundle (it lives on the agent sidecar and this path is never invoked on a Worker route), so just run the fn. Verified: `wrangler deploy --env production --dry-run` now builds clean (15.8 MB bundle, no missing-export error).
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
2 participants
Summary
Refs #11592 (auto-closed by #11493's merge — the core "stranded app-chat holds are never swept" gap IS fixed there and verified below). This PR fixes the residual money bug in the merged sweep's app-chat settle math plus real-PGlite proof that a stranded hold is swept exactly once (no double-refund).
Verified already fixed on develop (no duplication here)
settlement_marker = app_chat_reservation_v1(apps/[id]/chat/route.ts:326) and the hold txid is threaded through every settle/refund site.sweepStaleReservationsmatches both marker shapes, claims via compare-and-set onsettled_at, refunds under the idempotentrecon:<txid>:refundkey (cloud/money: createCreditReservationSettler reset-on-throw + non-idempotent reconcile refund → cashable double-mint on monetized-app inference #11512/Money-integrity: app-inference compensation doesn't reverse creator earnings + reconcile retry double-applies (2 findings) #10846 pattern).api/cron/sweep-credit-reservations) and fanned out on the* * * * *schedule inwrangler.toml.The residual bug (fixed here): unit mismatch → platform eats the creator markup
The sweep settles a stale hold to
metadata.estimated_cost. For generic reservations that's org-charge units — correct. For app-chat holds it's the unbuffered BASE cost (no markup), while the row amount is the org charge: buffered base plus creator markup (computeInferenceChargeviaappCreditsService.deductCredits) — and the creator's earnings are recorded at deduct time.Concretely ($1.00 base estimate, 1.5× buffer, 20% markup → $1.80 hold, $0.30 creator earnings):
Fix: for
app_chat_reservationrows, settle the org toamount × (estimated_cost / reserved_amount)— exactly what a normal settle at the estimated cost charges. Markup-0 (non-monetized) holds collapse to the previous math byte-for-byte; holds without a usable base pair settle exact-cost; result clamped to the held amount so corrupt metadata can never become a surprise overage charge. The pre-existing test masked the bug by modelingreserved_amount == row amount(markup = 0).Red / green (real PGlite, no mocks)
Pre-fix (develop
credits.ts+ the new test):Post-fix:
bun test --isolate --conditions eliza-source packages/cloud/shared/src/lib/services/__tests__/credits-reconcile.test.ts→ Ran 30 tests, 0 fail.New tests assert the exactly-once contract: single
recon:<txid>:refundrow (0.600000),settled_atclaimed, re-sweep scans 0, late full-refund settle after the sweep is a no-op (refund count stays 1) — plus missing-base-pair holds settle exact-cost instead of guessing.Biome: 2 files, no fixes. Typecheck: no errors for touched files. Evidence:
.github/issue-evidence/11592-app-chat-sweep-markup.md.Remaining scope (explicit, needs [cloud-money])
The sweep still doesn't reverse the creator-earnings share of the buffer delta (
(reservedBase − estimatedBase) × markup, $0.10 above): that requiresappCreditsServiceearnings machinery, whichcreditsServicecan't import (dependency direction — app-credits already imports credits). Platform-absorbed, bounded, rare-path (mid-settle-throw strands only). If wanted, the cron route (which can import both services) could route app-chat rows throughappCreditsService.reconcileCreditsinstead — a design call for the money lane, not snuck in here.Money path → requesting [cloud-money] / @lalalune review. Tracked in #8434.
— [cloud-security]