Skip to content

Commit 7c6ba8d

Browse files
fix(payments): page the payout ledger reads and verify they are complete (#533)
`getPayoutsOwed()` read `tenants`, `revenue_splits`, `transactions` and `payouts` with no `.range()` or `.limit()`. PostgREST caps every response at the project's configured API "Max rows" (1000, locally and on the cloud project) and reports the cap as an ordinary 200 with fewer rows, so `computeOwedBalances()` — pure arithmetic over whatever array it is handed — produced a confidently wrong balance rather than an error. A short `transactions` read underpays the school; a short `payouts` read overpays it. It was not display-only: `markPayoutPaid()` re-calls `getPayoutsOwed()` for its 10% mismatch guard, so a truncated balance also decided whether an operator was warned about a wrong payout amount. Add `lib/supabase/fetch-all-rows.ts`: pages a query until the relation is exhausted, then asserts the rows it collected against PostgREST's own exact count and throws — naming the relation and both numbers — on a shortfall. Fetching more than the first count is fine (rows inserted mid-sweep); only a shortfall means data was dropped. The helper does not depend on knowing the cap. PostgREST clamps ranged requests too, so a window wider than the cap comes back short without being the end of the relation — the loop adopts the size the server actually returned and continues. Verified against the local stack with the cap forced to 5: a bare `.select()` returned 5 of 20 rows with no error, while `fetchAllRows` returned all 20, unique and in order, still asking for 500-row pages. Paging rather than a SECURITY DEFINER aggregate keeps one source of truth: `payouts-owed.ts` carries the documented, test-covered rules (per-transaction split snapshot with its pre-#496 fallback, per-currency grouping, the reporting-only clawback heuristic, #516's overpaid term), and restating those in SQL would create two implementations that must agree forever. Same treatment for the `tenants` sweep in `enforce-plan-limits`, where a cap silently stopped the daily reconcile from visiting the tail of the tenant list — the only organic-growth trigger for #494's access cutoff. An incomplete read now fails the run with a 500 instead of under-reporting. Documents the rule and the confirmed cap in `docs/PLATFORM_ADMIN.md`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6Vfwhyj4Sktx8Am54cCGz
1 parent 52ab5e9 commit 7c6ba8d

5 files changed

Lines changed: 380 additions & 19 deletions

File tree

app/actions/platform/payouts.ts

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { revalidatePath } from 'next/cache'
77
import { getCurrentUserId } from '@/lib/supabase/tenant'
88
import { PROVIDER_CAPABILITIES, type PaymentProvider } from '@/lib/payments/types'
99
import { computeOwedBalances, DEFAULT_SCHOOL_PERCENTAGE, type TenantOwed } from '@/lib/payments/payouts-owed'
10+
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'
1011

1112
async function verifySuperAdmin() {
1213
const supabase = await createClient()
@@ -24,32 +25,57 @@ export async function getPayoutsOwed(): Promise<TenantOwed[]> {
2425
await verifySuperAdmin()
2526
const admin = createAdminClient()
2627

27-
const [{ data: tenants }, { data: splits }, { data: txns }, { data: paid }] = await Promise.all([
28-
admin.from('tenants').select('id, name'),
29-
admin.from('revenue_splits').select('tenant_id, school_percentage'),
30-
admin
31-
.from('transactions')
32-
.select('tenant_id, payment_provider, amount, currency, school_percentage_snapshot, status, transaction_date')
33-
.in('status', ['successful', 'refunded'])
34-
.in('payment_provider', PLATFORM_SETTLED_PROVIDERS),
35-
admin
36-
.from('payouts')
37-
.select('tenant_id, amount, currency, period_end, paid_at, created_at')
38-
.eq('payout_method', 'manual')
39-
.eq('status', 'paid'),
28+
// Every read here is a whole-relation sweep whose rows are then summed, so a
29+
// PostgREST row cap would misstate the balance instead of failing (issue
30+
// #533): a short `transactions` read underpays the school, a short `payouts`
31+
// read overpays it. `fetchAllRows` pages and then verifies the row count it
32+
// collected against the server's own — hence `{ count: 'exact' }` and the
33+
// primary-key `.order()` on each of the four.
34+
const [tenants, splits, txns, paid] = await Promise.all([
35+
fetchAllRows('tenants', (from, to) =>
36+
admin.from('tenants').select('id, name', { count: 'exact' }).order('id').range(from, to)
37+
),
38+
fetchAllRows('revenue_splits', (from, to) =>
39+
admin
40+
.from('revenue_splits')
41+
.select('tenant_id, school_percentage', { count: 'exact' })
42+
.order('split_id')
43+
.range(from, to)
44+
),
45+
fetchAllRows('transactions', (from, to) =>
46+
admin
47+
.from('transactions')
48+
.select(
49+
'tenant_id, payment_provider, amount, currency, school_percentage_snapshot, status, transaction_date',
50+
{ count: 'exact' }
51+
)
52+
.in('status', ['successful', 'refunded'])
53+
.in('payment_provider', PLATFORM_SETTLED_PROVIDERS)
54+
.order('transaction_id')
55+
.range(from, to)
56+
),
57+
fetchAllRows('payouts', (from, to) =>
58+
admin
59+
.from('payouts')
60+
.select('tenant_id, amount, currency, period_end, paid_at, created_at', { count: 'exact' })
61+
.eq('payout_method', 'manual')
62+
.eq('status', 'paid')
63+
.order('payout_id')
64+
.range(from, to)
65+
),
4066
])
4167

4268
const schoolPercentageByTenant = new Map(
43-
(splits || []).map((s) => [s.tenant_id, s.school_percentage as number])
69+
splits.map((s) => [s.tenant_id, s.school_percentage as number])
4470
)
4571

4672
return computeOwedBalances(
47-
(tenants || []).map((t) => ({
73+
tenants.map((t) => ({
4874
tenantId: t.id,
4975
tenantName: t.name,
5076
schoolPercentage: schoolPercentageByTenant.get(t.id) ?? DEFAULT_SCHOOL_PERCENTAGE,
5177
})),
52-
(txns || [])
78+
txns
5379
.filter((t) => t.tenant_id && t.payment_provider && t.amount != null)
5480
.map((t) => ({
5581
tenantId: t.tenant_id as string,
@@ -63,7 +89,7 @@ export async function getPayoutsOwed(): Promise<TenantOwed[]> {
6389
// `period_end` is the exact "covered through" when a payout recorded a period;
6490
// manually recorded ones leave it null today, so fall back to when the money
6591
// actually moved (issue #511).
66-
(paid || []).map((p) => ({
92+
paid.map((p) => ({
6793
tenantId: p.tenant_id,
6894
amount: p.amount,
6995
currency: p.currency || 'usd',

app/api/cron/enforce-plan-limits/route.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { NextRequest, NextResponse } from 'next/server'
22
import { createClient, type SupabaseClient } from '@supabase/supabase-js'
33
import { reconcileAccessCutoff } from '@/lib/billing/access-cutoff'
4+
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'
45

56
export const runtime = 'nodejs'
67

@@ -46,7 +47,24 @@ export async function GET(req: NextRequest) {
4647

4748
const supabase = getSupabaseAdmin()
4849

49-
const { data: tenants } = await supabase.from('tenants').select('id')
50+
// Paged and verified rather than read in one shot (issue #533). A PostgREST
51+
// row cap would truncate this list into a plain short array, and since the
52+
// sweep's only job is to visit EVERY tenant, the tail would simply stop being
53+
// reconciled with nothing in the response to say so — silently disabling the
54+
// organic-growth trigger this cron exists to be. An incomplete read now fails
55+
// the run loudly so the scheduler surfaces it.
56+
let tenants: { id: string }[]
57+
try {
58+
tenants = await fetchAllRows<{ id: string }>('tenants', (from, to) =>
59+
supabase.from('tenants').select('id', { count: 'exact' }).order('id').range(from, to)
60+
)
61+
} catch (err) {
62+
console.error('enforce-plan-limits: could not read the tenant list', err)
63+
return NextResponse.json(
64+
{ error: err instanceof Error ? err.message : 'Failed to read tenants' },
65+
{ status: 500 }
66+
)
67+
}
5068

5169
const result = {
5270
scheduled: 0,
@@ -59,7 +77,7 @@ export async function GET(req: NextRequest) {
5977
notifyFailures: 0,
6078
}
6179

62-
for (const tenant of tenants || []) {
80+
for (const tenant of tenants) {
6381
try {
6482
const decision = await reconcileAccessCutoff(supabase, tenant.id, { notifyDueStages: true })
6583
if (decision.action === 'schedule') result.scheduled++

docs/PLATFORM_ADMIN.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,31 @@ const adminClient = createAdminClient()
7070

7171
**Important gotcha:** PostgREST FK embedding (`table(related_col)`) silently fails with the admin client for some FK relationships. Always fetch related records in a separate query using `.in('id', ids)`.
7272

73+
### Platform-wide sweeps must be paged (issue #533)
74+
75+
PostgREST caps every response at the project's configured API **"Max rows"**. `supabase/config.toml` sets `max_rows = 1000` for the local stack; the cloud project has no `pgrst.db_max_rows` role override, so it runs on Supabase's hosted default, also **1000**. A capped response is an ordinary `200` with fewer rows in it — there is no error, no flag, and nothing downstream can tell it apart from a complete result.
76+
77+
That makes it lethal for platform pages, which read whole cross-tenant relations and then do arithmetic over them. `getPayoutsOwed()` used to read `transactions` and `payouts` unbounded: past 1000 rows it would have underpaid or overpaid schools by however much fell off the end, confidently and silently.
78+
79+
So: **any read whose rows are summed, counted, or iterated for side effects must use `fetchAllRows()`** (`lib/supabase/fetch-all-rows.ts`) rather than a bare `.select()`.
80+
81+
```ts
82+
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'
83+
84+
const rows = await fetchAllRows('transactions', (from, to) =>
85+
admin
86+
.from('transactions')
87+
.select('tenant_id, amount', { count: 'exact' }) // required — the completeness check compares against it
88+
.eq('status', 'successful')
89+
.order('transaction_id') // required — a unique key, or pages overlap/skip
90+
.range(from, to)
91+
)
92+
```
93+
94+
It pages until the relation is exhausted, shrinks its page size to whatever the server actually returns (so it is correct for any cap without knowing the number), and throws — naming the relation and both counts — if it ends up with fewer rows than the server said exist. A wrong total is worse than a failed page, so it fails.
95+
96+
Reads that are already bounded (`.limit()`, `.single()`, a tenant-scoped page query) don't need it, and neither do aggregations done in SQL: `get_platform_stats()` and `get_platform_revenue()` sum inside Postgres and return one row, which is immune by construction and remains the better option for a large relation.
97+
7398
---
7499

75100
## Feature Areas

lib/supabase/fetch-all-rows.ts

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/**
2+
* Complete, verified reads for platform-wide sweeps (issue #533).
3+
*
4+
* PostgREST caps every response at the project's configured API "Max rows"
5+
* (`supabase/config.toml:18` sets `max_rows = 1000` locally; the cloud project
6+
* carries no `pgrst.db_max_rows` role override, so it runs on Supabase's
7+
* hosted default of 1000). A capped response is a plain 200 with fewer rows —
8+
* indistinguishable from a complete one. Any caller that reads a whole
9+
* relation and then does arithmetic over it therefore produces a confidently
10+
* wrong number rather than an error: `getPayoutsOwed()` underpaid a school
11+
* when `transactions` truncated and overpaid it when `payouts` did, and the
12+
* `enforce-plan-limits` cron silently stopped visiting tenants past the cap.
13+
*
14+
* `fetchAllRows` pages until the relation is exhausted and then ASSERTS the
15+
* result is complete against PostgREST's own exact count, so a cap (or a
16+
* future change to it) surfaces as a thrown error instead of a quiet
17+
* shortfall. Nothing here depends on the cap's value: the page size is ours,
18+
* and the assertion is what actually guarantees completeness.
19+
*/
20+
21+
/**
22+
* Rows requested per page. Deliberately below the 1000 cap so the common case
23+
* never brushes against it — but the loop does not rely on that: a server
24+
* configured lower simply hands back shorter pages, and `fetchAllRows` adopts
25+
* the size it actually got (see below) rather than mistaking a capped page for
26+
* the end of the relation. Nothing here breaks if the cap is changed.
27+
*/
28+
export const FETCH_ALL_PAGE_SIZE = 500
29+
30+
/**
31+
* Runaway guard — a backstop against a server that keeps returning full pages
32+
* forever, and against a pathologically small cap turning a big relation into
33+
* an unbounded request storm. At the default page size this covers 1,000,000
34+
* rows, far past anything these sweeps read.
35+
*/
36+
const MAX_PAGES = 2000
37+
38+
/** The shape of a `count: 'exact'` PostgREST response, narrowed to what this helper reads. */
39+
interface CountedPage<T> {
40+
data: T[] | null
41+
error: { message: string } | null
42+
count: number | null
43+
}
44+
45+
/**
46+
* Read every row of a query, verifying nothing was silently dropped.
47+
*
48+
* The caller builds one page per call. Two requirements, both load-bearing:
49+
*
50+
* - **`{ count: 'exact' }` on the select.** It is what the completeness
51+
* assertion compares against; without it there is nothing to verify and the
52+
* helper degrades to a plain paging loop.
53+
* - **A stable `.order()` on a unique column** (a primary key). `.range()`
54+
* windows over an unordered relation may overlap or skip rows between
55+
* requests, which would trade a truncation bug for a duplication bug.
56+
*
57+
* ```ts
58+
* const txns = await fetchAllRows('transactions', (from, to) =>
59+
* admin
60+
* .from('transactions')
61+
* .select('tenant_id, amount', { count: 'exact' })
62+
* .eq('status', 'successful')
63+
* .order('transaction_id')
64+
* .range(from, to)
65+
* )
66+
* ```
67+
*
68+
* @param relation Name used in error messages — the operator needs to know
69+
* which read came up short.
70+
* @throws if a page errors, if the relation is larger than `MAX_PAGES` pages,
71+
* or if fewer rows were collected than the server said exist.
72+
*/
73+
export async function fetchAllRows<T>(
74+
relation: string,
75+
page: (from: number, to: number) => PromiseLike<CountedPage<T>>,
76+
pageSize: number = FETCH_ALL_PAGE_SIZE,
77+
): Promise<T[]> {
78+
const rows: T[] = []
79+
/**
80+
* The count reported by the FIRST page — a snapshot of the relation as the
81+
* sweep started. Later pages report a moving total under concurrent writes;
82+
* comparing against the snapshot keeps the assertion about truncation rather
83+
* than about how busy the table is.
84+
*/
85+
let expected: number | null = null
86+
/**
87+
* Shrinks to whatever the server is actually willing to return. PostgREST
88+
* applies "Max rows" to each ranged request too, so asking for a window
89+
* wider than the cap yields a short page that is NOT the end of the relation.
90+
* Adopting the observed size makes the loop correct for any cap without
91+
* knowing its value.
92+
*/
93+
let effectivePageSize = pageSize
94+
95+
for (let request = 0; request < MAX_PAGES; request++) {
96+
// Offset by rows already collected rather than by page index: it stays
97+
// correct after `effectivePageSize` shrinks mid-sweep.
98+
const from = rows.length
99+
const { data, error, count } = await page(from, from + effectivePageSize - 1)
100+
if (error) {
101+
throw new Error(`fetchAllRows(${relation}): page at offset ${from} failed — ${error.message}`)
102+
}
103+
if (request === 0) expected = count
104+
105+
const batch = data ?? []
106+
rows.push(...batch)
107+
108+
if (batch.length < effectivePageSize) {
109+
// Short page. Either the relation is exhausted, or the server capped this
110+
// request below what we asked for. The count settles it: if rows are
111+
// still owed and the server just proved it can return `batch.length` at a
112+
// time, keep going at that size. An empty page means it can return
113+
// nothing more, and the assertion below turns that into a loud error.
114+
if (expected != null && rows.length < expected && batch.length > 0) {
115+
effectivePageSize = batch.length
116+
continue
117+
}
118+
assertComplete(relation, rows.length, expected)
119+
return rows
120+
}
121+
// An exactly-full page is ambiguous — the next one may be empty — so we
122+
// always ask again. The trailing empty request is expected, not wasteful.
123+
}
124+
125+
throw new Error(
126+
`fetchAllRows(${relation}): still returning full pages after ${MAX_PAGES} requests ` +
127+
`(${rows.length} rows); refusing to loop further.`,
128+
)
129+
}
130+
131+
/**
132+
* The last line of defence. Collecting FEWER rows than the server itself
133+
* reported is the failure this module exists to catch: the loop above handles
134+
* the ordinary row cap, so a shortfall here means something else ate rows — the
135+
* server refusing to return any more, or rows deleted mid-sweep shifting the
136+
* offsets. Either way the caller's totals would be wrong, so it gets an error
137+
* instead of a number.
138+
*
139+
* Collecting MORE is benign: rows inserted while the sweep ran. Treating that
140+
* as corruption would fail the platform payouts page whenever a sale lands
141+
* mid-render.
142+
*/
143+
function assertComplete(relation: string, fetched: number, expected: number | null) {
144+
if (expected == null) return
145+
if (fetched < expected) {
146+
throw new Error(
147+
`fetchAllRows(${relation}): incomplete read — fetched ${fetched} of ${expected} rows. ` +
148+
`Any total computed from this would be silently wrong, so nothing is returned.`,
149+
)
150+
}
151+
}

0 commit comments

Comments
 (0)