Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 43 additions & 17 deletions app/actions/platform/payouts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { revalidatePath } from 'next/cache'
import { getCurrentUserId } from '@/lib/supabase/tenant'
import { PROVIDER_CAPABILITIES, type PaymentProvider } from '@/lib/payments/types'
import { computeOwedBalances, DEFAULT_SCHOOL_PERCENTAGE, type TenantOwed } from '@/lib/payments/payouts-owed'
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'

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

const [{ data: tenants }, { data: splits }, { data: txns }, { data: paid }] = await Promise.all([
admin.from('tenants').select('id, name'),
admin.from('revenue_splits').select('tenant_id, school_percentage'),
admin
.from('transactions')
.select('tenant_id, payment_provider, amount, currency, school_percentage_snapshot, status, transaction_date')
.in('status', ['successful', 'refunded'])
.in('payment_provider', PLATFORM_SETTLED_PROVIDERS),
admin
.from('payouts')
.select('tenant_id, amount, currency, period_end, paid_at, created_at')
.eq('payout_method', 'manual')
.eq('status', 'paid'),
// Every read here is a whole-relation sweep whose rows are then summed, so a
// PostgREST row cap would misstate the balance instead of failing (issue
// #533): a short `transactions` read underpays the school, a short `payouts`
// read overpays it. `fetchAllRows` pages and then verifies the row count it
// collected against the server's own — hence `{ count: 'exact' }` and the
// primary-key `.order()` on each of the four.
const [tenants, splits, txns, paid] = await Promise.all([
fetchAllRows('tenants', (from, to) =>
admin.from('tenants').select('id, name', { count: 'exact' }).order('id').range(from, to)
),
fetchAllRows('revenue_splits', (from, to) =>
admin
.from('revenue_splits')
.select('tenant_id, school_percentage', { count: 'exact' })
.order('split_id')
.range(from, to)
),
fetchAllRows('transactions', (from, to) =>
admin
.from('transactions')
.select(
'tenant_id, payment_provider, amount, currency, school_percentage_snapshot, status, transaction_date',
{ count: 'exact' }
)
.in('status', ['successful', 'refunded'])
.in('payment_provider', PLATFORM_SETTLED_PROVIDERS)
.order('transaction_id')
.range(from, to)
),
fetchAllRows('payouts', (from, to) =>
admin
.from('payouts')
.select('tenant_id, amount, currency, period_end, paid_at, created_at', { count: 'exact' })
.eq('payout_method', 'manual')
.eq('status', 'paid')
.order('payout_id')
.range(from, to)
),
])

const schoolPercentageByTenant = new Map(
(splits || []).map((s) => [s.tenant_id, s.school_percentage as number])
splits.map((s) => [s.tenant_id, s.school_percentage as number])
)

return computeOwedBalances(
(tenants || []).map((t) => ({
tenants.map((t) => ({
tenantId: t.id,
tenantName: t.name,
schoolPercentage: schoolPercentageByTenant.get(t.id) ?? DEFAULT_SCHOOL_PERCENTAGE,
})),
(txns || [])
txns
.filter((t) => t.tenant_id && t.payment_provider && t.amount != null)
.map((t) => ({
tenantId: t.tenant_id as string,
Expand All @@ -63,7 +89,7 @@ export async function getPayoutsOwed(): Promise<TenantOwed[]> {
// `period_end` is the exact "covered through" when a payout recorded a period;
// manually recorded ones leave it null today, so fall back to when the money
// actually moved (issue #511).
(paid || []).map((p) => ({
paid.map((p) => ({
tenantId: p.tenant_id,
amount: p.amount,
currency: p.currency || 'usd',
Expand Down
22 changes: 20 additions & 2 deletions app/api/cron/enforce-plan-limits/route.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { NextRequest, NextResponse } from 'next/server'
import { createClient, type SupabaseClient } from '@supabase/supabase-js'
import { reconcileAccessCutoff } from '@/lib/billing/access-cutoff'
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'

export const runtime = 'nodejs'

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

const supabase = getSupabaseAdmin()

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

const result = {
scheduled: 0,
Expand All @@ -59,7 +77,7 @@ export async function GET(req: NextRequest) {
notifyFailures: 0,
}

for (const tenant of tenants || []) {
for (const tenant of tenants) {
try {
const decision = await reconcileAccessCutoff(supabase, tenant.id, { notifyDueStages: true })
if (decision.action === 'schedule') result.scheduled++
Expand Down
25 changes: 25 additions & 0 deletions docs/PLATFORM_ADMIN.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,31 @@ const adminClient = createAdminClient()

**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)`.

### Platform-wide sweeps must be paged (issue #533)

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.

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.

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()`.

```ts
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'

const rows = await fetchAllRows('transactions', (from, to) =>
admin
.from('transactions')
.select('tenant_id, amount', { count: 'exact' }) // required — the completeness check compares against it
.eq('status', 'successful')
.order('transaction_id') // required — a unique key, or pages overlap/skip
.range(from, to)
)
```

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.

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.

---

## Feature Areas
Expand Down
151 changes: 151 additions & 0 deletions lib/supabase/fetch-all-rows.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/**
* Complete, verified reads for platform-wide sweeps (issue #533).
*
* PostgREST caps every response at the project's configured API "Max rows"
* (`supabase/config.toml:18` sets `max_rows = 1000` locally; the cloud project
* carries no `pgrst.db_max_rows` role override, so it runs on Supabase's
* hosted default of 1000). A capped response is a plain 200 with fewer rows —
* indistinguishable from a complete one. Any caller that reads a whole
* relation and then does arithmetic over it therefore produces a confidently
* wrong number rather than an error: `getPayoutsOwed()` underpaid a school
* when `transactions` truncated and overpaid it when `payouts` did, and the
* `enforce-plan-limits` cron silently stopped visiting tenants past the cap.
*
* `fetchAllRows` pages until the relation is exhausted and then ASSERTS the
* result is complete against PostgREST's own exact count, so a cap (or a
* future change to it) surfaces as a thrown error instead of a quiet
* shortfall. Nothing here depends on the cap's value: the page size is ours,
* and the assertion is what actually guarantees completeness.
*/

/**
* Rows requested per page. Deliberately below the 1000 cap so the common case
* never brushes against it — but the loop does not rely on that: a server
* configured lower simply hands back shorter pages, and `fetchAllRows` adopts
* the size it actually got (see below) rather than mistaking a capped page for
* the end of the relation. Nothing here breaks if the cap is changed.
*/
export const FETCH_ALL_PAGE_SIZE = 500

/**
* Runaway guard — a backstop against a server that keeps returning full pages
* forever, and against a pathologically small cap turning a big relation into
* an unbounded request storm. At the default page size this covers 1,000,000
* rows, far past anything these sweeps read.
*/
const MAX_PAGES = 2000

/** The shape of a `count: 'exact'` PostgREST response, narrowed to what this helper reads. */
interface CountedPage<T> {
data: T[] | null
error: { message: string } | null
count: number | null
}

/**
* Read every row of a query, verifying nothing was silently dropped.
*
* The caller builds one page per call. Two requirements, both load-bearing:
*
* - **`{ count: 'exact' }` on the select.** It is what the completeness
* assertion compares against; without it there is nothing to verify and the
* helper degrades to a plain paging loop.
* - **A stable `.order()` on a unique column** (a primary key). `.range()`
* windows over an unordered relation may overlap or skip rows between
* requests, which would trade a truncation bug for a duplication bug.
*
* ```ts
* const txns = await fetchAllRows('transactions', (from, to) =>
* admin
* .from('transactions')
* .select('tenant_id, amount', { count: 'exact' })
* .eq('status', 'successful')
* .order('transaction_id')
* .range(from, to)
* )
* ```
*
* @param relation Name used in error messages — the operator needs to know
* which read came up short.
* @throws if a page errors, if the relation is larger than `MAX_PAGES` pages,
* or if fewer rows were collected than the server said exist.
*/
export async function fetchAllRows<T>(
relation: string,
page: (from: number, to: number) => PromiseLike<CountedPage<T>>,
pageSize: number = FETCH_ALL_PAGE_SIZE,
): Promise<T[]> {
const rows: T[] = []
/**
* The count reported by the FIRST page — a snapshot of the relation as the
* sweep started. Later pages report a moving total under concurrent writes;
* comparing against the snapshot keeps the assertion about truncation rather
* than about how busy the table is.
*/
let expected: number | null = null
/**
* Shrinks to whatever the server is actually willing to return. PostgREST
* applies "Max rows" to each ranged request too, so asking for a window
* wider than the cap yields a short page that is NOT the end of the relation.
* Adopting the observed size makes the loop correct for any cap without
* knowing its value.
*/
let effectivePageSize = pageSize

for (let request = 0; request < MAX_PAGES; request++) {
// Offset by rows already collected rather than by page index: it stays
// correct after `effectivePageSize` shrinks mid-sweep.
const from = rows.length
const { data, error, count } = await page(from, from + effectivePageSize - 1)
if (error) {
throw new Error(`fetchAllRows(${relation}): page at offset ${from} failed — ${error.message}`)
}
if (request === 0) expected = count

const batch = data ?? []
rows.push(...batch)

if (batch.length < effectivePageSize) {
// Short page. Either the relation is exhausted, or the server capped this
// request below what we asked for. The count settles it: if rows are
// still owed and the server just proved it can return `batch.length` at a
// time, keep going at that size. An empty page means it can return
// nothing more, and the assertion below turns that into a loud error.
if (expected != null && rows.length < expected && batch.length > 0) {
effectivePageSize = batch.length
continue
}
assertComplete(relation, rows.length, expected)
return rows
}
// An exactly-full page is ambiguous — the next one may be empty — so we
// always ask again. The trailing empty request is expected, not wasteful.
}

throw new Error(
`fetchAllRows(${relation}): still returning full pages after ${MAX_PAGES} requests ` +
`(${rows.length} rows); refusing to loop further.`,
)
}

/**
* The last line of defence. Collecting FEWER rows than the server itself
* reported is the failure this module exists to catch: the loop above handles
* the ordinary row cap, so a shortfall here means something else ate rows — the
* server refusing to return any more, or rows deleted mid-sweep shifting the
* offsets. Either way the caller's totals would be wrong, so it gets an error
* instead of a number.
*
* Collecting MORE is benign: rows inserted while the sweep ran. Treating that
* as corruption would fail the platform payouts page whenever a sale lands
* mid-render.
*/
function assertComplete(relation: string, fetched: number, expected: number | null) {
if (expected == null) return
if (fetched < expected) {
throw new Error(
`fetchAllRows(${relation}): incomplete read — fetched ${fetched} of ${expected} rows. ` +
`Any total computed from this would be silently wrong, so nothing is returned.`,
)
}
}
Loading
Loading