Re-review: admin personas + install wizard + mail UI + Network carve - #9
Open
keithfawcett wants to merge 259 commits into
Open
Re-review: admin personas + install wizard + mail UI + Network carve#9keithfawcett wants to merge 259 commits into
keithfawcett wants to merge 259 commits into
Conversation
New Settings page at /admin/settings lets the admin edit runtime
content without shell access:
- Program name — replaces "OpenPartner" in the sidebar brand
- Support email — rendered in the sidebar footer as a mailto:
link for partners to contact
Backed by the Config table, keyed 'program_settings'. GET /config/program
is auth-only (admin + partner sessions can both read). POST is admin
only. Partner portal fetches on mount with a 60s staleTime so edits
propagate quickly without hammering the endpoint.
Admin's Partners table now wraps each partner's email in mailto: too,
so one click opens a reply with that partner.
Env remains reserved for secrets + build-time properties; everything
user-facing goes through this pattern instead.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three tightly-coupled changes:
1. Admin as a first-class persona (not just ADMIN_API_KEY)
- New Admin table (id, email, name, activatedAt, revokedAt,
revokeReason, lastSignInAt).
- Session + MagicLinkToken generalized from partnerId to
(principalKind, principalId) so the same magic-link infra carries
both admins and partners. Backfill stamps existing rows
principalKind='partner'.
- Unified /auth/signin and /auth/magic/verify branch on the token's
principalKind — admins get adminInviteEmail + adminSigninEmail,
partners get their existing templates.
- /admins routes: list, invite, resend, revoke, reinstate. Revoke
has two guardrails: can't revoke yourself (session-sourced), and
can't drop active-admin count to zero.
- ADMIN_API_KEY env stays as bootstrap / headless / CI path; human
admins sign in via the account instead.
- Portal gets an Admins page under Admin; principal chip shows
"admin (env)" for env-bearer, admin name for session admins.
2. SMTP support via nodemailer
- Mailer auto-select: SMTP_HOST + MAIL_FROM → SMTP, else
POSTMARK_SERVER_TOKEN + MAIL_FROM → Postmark, else console
fallback (dev only).
- SMTP covers the universe of providers (Gmail, Workspace, SES,
Mailgun, SendGrid, Resend, Postmark SMTP, self-hosted Postfix).
- Postmark stays as a dedicated adapter.
- .env.example documents the two transport options.
3. /install wizard — WordPress-style first-run
- New InstallPage at /install, unauthenticated. Single form
collects admin name+email + program name + support email.
- GET /install/status lets the portal route to /install while
needsSetup=true and back to normal once an admin is activated.
- POST /install is rate-limited and refuses (409) once any active
admin exists — second installer can't take over.
- Submit creates the Admin row + saves program_settings +
sends the magic-link invite in one round-trip.
New tests (4) cover: install → first-admin happy path; admin invite +
signin; last-active-admin revoke guard; duplicate-email 409. Partners'
existing revoke had to switch Session lookup from partnerId to
(principalKind, principalId) too.
56 api / 3 router / 11 sdk = 70 tests, all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mail transport + credentials now live in the Config table and the
admin edits them from Settings → Email delivery. Rotating SMTP
passwords or Postmark tokens no longer requires a redeploy.
Encryption at rest:
- New SECRETS_ENCRYPTION_KEY env (32 bytes hex/base64), required in
production. Dev uses a fixed fallback with a warning.
- AES-256-GCM envelope (12-byte IV + 16-byte tag + ciphertext, base64).
- Only secret fields are encrypted (SMTP password, Postmark token).
Host/port/user/from stay plaintext — identifiers, not secrets.
- Public readers (the Settings UI) get a sanitized view: `hasPassword`
/ `hasToken` booleans instead of plaintext.
Resolution order at dispatch time:
1. UI-configured settings (Config table) win
2. Env vars (SMTP_HOST / POSTMARK_SERVER_TOKEN + MAIL_FROM) as fallback
3. Console (dev only)
Install wizard grows an Email delivery step: provider (SMTP / Postmark
/ None), from address, and all the fields for the chosen provider.
Settings page mirrors it with "saved ✓" indicators on fields whose
secret is already stored (leave blank to keep, enter to rotate).
The mailer now creates a transporter per-send so rotating credentials
from the UI takes effect on the next email without a restart. Tests
keep injecting a capturing mailer via __setMailerForTests.
.do/app.yaml, docker-compose.prod.yml, ci.yml get SECRETS_ENCRYPTION_KEY
wired in. .env.example rewrites the mail section to explain the UI-first
flow with env as fallback.
56 api / 3 router / 11 sdk tests all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The single-form install grew oppressive. Break it into YOU → PROGRAM → EMAIL DELIVERY with a stepper up top, Back/Continue navigation, and the final step submitting. Validation is per-step (can't advance without the required fields for that step). Same submit body; only the UX changed.
README adds a What's implemented subsection grouping by area (attribution + payouts, auth + personas, configuration, integration surface, operations). Quickstart leads with the /install wizard and keeps the curl bootstrap as a headless / CI alternative rather than the default. ARCHITECTURE gains sections on personas (Admin + Partner, magic-link auth, revoke semantics) and Settings + secret encryption. docs/deploy.md now calls out SECRETS_ENCRYPTION_KEY as a required production env and documents that mail env vars are fallbacks rather than the primary path.
…race Three security / lockout fixes from the ultrareview re-pass (PR #9): 1. POST /auth/signin for a revoked partner was sending the partner_revoked notice email every time. An attacker could spam any known partner's inbox by POSTing their email repeatedly. Now: signin for a revoked partner is silent (no email). Revoke flow already sends a one-time notification at revoke time with the admin-provided reason — that's the only on-the-record touchpoint. 2. POST /install's "no admin exists" check was outside the tx. Two concurrent installers could both pass the check and both create admin rows. Now the check happens inside a transaction under a pg_advisory_xact_lock keyed to the install path, so concurrent calls serialize and the loser 409s. Also tightened the guard to block on ANY admin row (not just activated) so a second installer can't slip in while the first magic-link is still pending. 3. POST /admins/:id/revoke's "last active admin" guard was outside the transaction. Two concurrent revokes of the only two active admins both saw count=2, both proceeded, and both committed — leaving zero admins and locking everyone out. Now the guard runs inside a transaction with SELECT ... FOR UPDATE on the active- admins set, so concurrent revokes serialize and the loser 409s on cannot_revoke_last_active_admin. Also added revoked-admin guard to /admins/:id/invite (can't resend to a revoked account) and cleaned up the unused activeAdminCount helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four correctness fixes from ultrareview PR #9: - Partner revocation now also flips revokedAt on every ApiKey row tied to that partner. Previously sessions were killed but partner- scoped API keys lived on, so a revoked partner kept programmatic access via the SDK / curl. - POST /install is retryable on partial failure. The admin row is created inside a tx; the following mail-config save + invite email happen in a compensating try/catch that undoes the admin + magic-link token on any downstream error. Without this, a Postmark outage during first install left the instance permanently 409'd. - Install schema now rejects mail.kind='smtp'|'postmark' without a from address (plus host/serverToken for the respective transport). Previously the install "succeeded" and then the activation email silently failed because the mailer had no sender. - attribution.ts comment updated to match behavior: revoked-partner filtering skips them regardless of event timing, for both live event webhooks and backlog replays. Earlier the docstring suggested "events after revokedAt" but the code filtered more aggressively. Code is what we want; comment was lying. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four validation / UX fixes from ultrareview PR #9: - POST /partners pre-checks for a duplicate email and 409s with email_taken instead of letting the unique-constraint violation surface as a generic 500. A race path (two concurrent inserts passing the pre-check) is caught via pg error code 23505 and also maps to 409. - saveMailSettings now refuses kind='smtp' with an empty host and kind='postmark' without a serverToken. Both used to save garbage that would blow up at first email. MailSettingsValidationError maps to a 400 with a field pointer in the /config/mail route. - MagicLanding invalidates the install-status react-query cache on successful verify so the first-run admin lands on / after clicking their activation link. Before, the cache was keyed staleTime:Infinity and still said needsSetup=true from boot — / would redirect right back to /install until a hard refresh. - admin_accounts migration's down() dropped a non-existent index on MagicLinkToken (up() only ever added the index on Session). Dropped the stray dropIndex call so rollbacks succeed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Newly-activated partners used to land on an empty Dashboard with nothing to do. The card fills that gap — two checklist rows that auto-complete as the partner finishes each step, and the card disappears once both are done: ☐ Create your first share link → /links ☐ Connect Stripe to get paid → /connect Live from the partner's existing data: link count via GET /partners/:id/links, Stripe Connect readiness via /connect/status. The existing ConnectNudge (shown when there's actually money waiting to be paid out) suppresses while the checklist is up — one call to action at a time. Also removed the dead Network-role redirect in the Dashboard component (network_creator / network_vendor roles no longer exist since the Network carve-out).
Adds a Rewardful-style integration path where merchants pass the partner ref through Stripe Checkout's client_reference_id instead of calling op.identify() from their app. On checkout.session.completed we stitch an Identity (cref → customer.id) and emit signup; downstream invoice.paid and customer.subscription.created resolve through the existing path. Disambiguator: the existing handleConnectEvent for checkout.session.completed now skips when client_reference_id is set, so merchant→customer checkouts can't accidentally clobber the merchant's own subscription record. resolveUserIdFromCustomer falls back to an Identity-table lookup when metadata is missing — covers the race where invoice.paid lands before our metadata backfill propagates on Stripe's side. SDK: getReferral() helper with the canonical Stripe Checkout usage in the docstring. Aliases getCref() so existing integrations keep working. Tests: 5 cases — valid stitch, unknown cref dropped, idempotency on Stripe event-id retries, invoice.paid resolves via the stitched Identity, and the merchant-subscription path still fires when no client_reference_id is set. Stripe customer ops mocked via vi.mock; signature verification real. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stripe's Event destinations UI splits platform-account events (checkout.*, customer.*, invoice.*) and connected-account events (account.updated, transfer.*) into separate destinations, each with its own signing secret. Both destinations point at the same /webhooks/stripe URL, so we just need to verify against any configured secret. STRIPE_WEBHOOK_SECRET now accepts either a single secret (existing behavior) or a comma-separated list. We try each in turn until one verifies; if none do, return 400 invalid_signature as before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
One-shot, idempotent script that creates OpenPartner Flex, Network access, and Revshare products in any Stripe account. Outputs the STRIPE_FLAT_PRICE_ID value to add to .env. Run with: STRIPE_SECRET_KEY=sk_test_... node apps/api/scripts/setup-stripe.mjs Re-runs are safe — products are looked up by metadata key before create, and prices by amount + recurrence kind. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the major payments-readiness gaps before deploy.
Refund + reversal handling
- Map charge.refunded → reverse non-paid Commissions linked to the
original invoice via metadata.stripeInvoiceId. Already-paid
Commissions stay paid and the count surfaces on the refund Event for
admin attention (no automated clawback in v1).
- Map charge.dispute.created and invoice.payment_failed → corrective
audit Events; no auto-reversal (disputes can be won; failed invoices
never fired invoice.paid in the first place).
- Skip attribution on corrective event types so we don't create phantom
negative commissions.
Metered usage billing
- setup-stripe.mjs provisions Stripe Meters (openpartner_attributed_gmv,
openpartner_network_payouts) and metered Prices for Flex (1.5%),
Revshare (3%), and Network access (3%).
- usage-billing.ts aggregates attributed GMV between the last-reported
high-water mark and now, then reports to Stripe Billing Meter Events.
Idempotent via Stripe identifier; high-water mark only advances on
success.
- POST /billing/report-usage triggers manual reporting (admin scope).
Cron entry can be wired up later.
V2 Accounts Checkout fix
- Stripe Accounts V2 in test mode rejects Checkout sessions without a
pre-created Customer. /billing/checkout now creates (and reuses) a
Customer on first call, stamps it on Config, then passes it to
Checkout. Same record is used by Customer Portal after subscription.
Other
- /billing/checkout supports both flat (base + metered) and revshare
(metered-only) modes. Line items differ; same Customer reuse logic.
- Fix setConfig: jsonb column rejects raw strings; serialize through
JSON.stringify and cast for primitive values.
- Tests force OPENPARTNER_MODE=selfhost so vitest's auto-loaded .env
can't bleed in.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- scheduler.ts: croner-based, runs usage-report (daily 03:15 UTC) and runPayouts (Mon 09:00 UTC). Gated behind OPENPARTNER_ENABLE_SCHEDULER=1 so dev/test/CI don't fire scheduled jobs unexpectedly. protect: true prevents overlap when a job runs longer than its interval. - .do/app.yaml: adds STRIPE_FLAT_USAGE_PRICE_ID, REVSHARE/NETWORK price ids, and OPENPARTNER_ENABLE_SCHEDULER=1. Postgres flipped to production: true (paid tier, daily backups). - docs/deploy-production.md: end-to-end runbook for first launch on DO App Platform — secrets, DNS, Stripe webhook destinations, smoke checks, troubleshooting. Verified the api Docker image builds and runs cleanly against a fresh empty Postgres: all 19 migrations apply, /health returns 200, scheduler correctly logs its disabled state in dev. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rather than provisioning a dedicated managed Postgres cluster, OpenPartner
points at an existing cluster (e.g., a separate database inside the
Coherence cluster). DATABASE_URL becomes a SECRET env var on api + router
instead of a templated reference to the embedded ${openpartner-db.DATABASE_URL}.
Block is preserved in a comment so re-enabling a dedicated cluster later
is a paste-back rather than a recall.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds github source ref to each component (api, router, portal) and fixes the ingress rules: portal routes / by default, api gets /api, router gets /r as a path prefix until the dedicated subdomain is wired. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DO Managed Postgres serves a CA-signed cert that's not in Node's default trust store. With pg-node, sslmode=require alone fails the chain check and the migration runner aborts before the api can start. Adds sslFromConnectionString helper used by both the runtime db factory and the knex migration runner. Maps: sslmode=require | no-verify → encrypted, rejectUnauthorized=false sslmode=verify-ca | verify-full → encrypted, full chain check no sslmode → no ssl (local dev) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pg-connection-string >= v2.7 treats sslmode=require as verify-full and
overrides our explicit { rejectUnauthorized: false } config when both
are present (URL parsing happens after our config is set, so it wins).
Strip sslmode from the URL when we manage ssl ourselves. The connection
remains TLS-encrypted; we just opt out of the chain check that would
otherwise fail on managed providers' self-signed CAs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(db): multi-tenant foundation + RLS
Three new migrations and matching type updates lay the groundwork for
multi-tenant deployments while keeping single-tenant self-host working
identically (just with tenantId='default' baked in).
20260507000000_multi_tenant
- New Tenant table; seeded 'default' tenant.
- tenantId column on every data table (Partner, Campaign, Link, Click,
Identity, Event, Attribution, Commission, Payout, ApiKey, Config,
Admin, MagicLinkToken, Session, WebhookEndpoint, WebhookDelivery).
- Backfill existing rows to the default tenant.
- Re-scope unique constraints to be per-tenant: Partner.email,
Admin.email, Link.linkKey, Config.(key→tenantId,key).
20260507010000_rls_policies
- PlatformAdmin table (cross-tenant Coherence support staff).
- RLS ENABLE + FORCE on every tenanted table.
- Policy: row visible iff tenantId matches `app.tenant_id` GUC OR
`app.platform_admin` GUC = 'on'.
- Tenant table: row visible iff its id matches app.tenant_id (or
platform admin). Same for PlatformAdmin.
- Policies use COALESCE / current_setting(..., true) so an unset GUC
returns 0 rows (default deny) instead of erroring.
20260507020000_app_role
- Provisions a non-superuser openpartner_app role from
OPENPARTNER_APP_DB_PASSWORD. Postgres bypasses RLS for superusers
and BYPASSRLS roles regardless of FORCE, so RLS only protects when
the app connects as a constrained role.
- Grants DML (no DDL) on every tenanted table.
- Idempotent: rotates password if the role already exists.
- Skipped (with notice) when OPENPARTNER_APP_DB_PASSWORD is unset —
self-host installs that don't need RLS isolation can run the app as
the same role as migrations.
Migration runner sets `row_security = off` at session start so DDL
runs unrestricted.
Verified: connecting as openpartner_app, queries return 0 rows when
app.tenant_id is unset or mismatched, and only the in-scope tenant's
rows when set correctly. Platform-admin override works.
Types: every Row interface gained `tenantId: string`; new TenantRow,
PlatformAdminRow types and DEFAULT_TENANT_ID constant.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(api): tenancy middleware + connection split (admin vs app pools)
Two knex instances now:
db (admin pool, DATABASE_URL)
- migrations, signup, platform-admin tooling, jobs that need
cross-tenant access
- bypasses RLS (superuser/owner role)
appDb (app pool, DATABASE_URL_APP if set, else DATABASE_URL)
- normal request handling. When pointed at the openpartner_app role
every query is subject to RLS.
- per-request transaction in tenancy middleware sets
`app.tenant_id` (and optionally `app.platform_admin = 'on'`)
so RLS policies match correctly.
OPENPARTNER_TENANCY env (defaults 'single'):
single — every request runs as tenantId = DEFAULT_TENANT_ID. Self-host.
multi — path-based tenant resolution (/t/<slug>/...). Reserved
slugs (www, api, app, signup, etc.) reject.
tenantMiddleware:
- resolves tenantId for the request
- opens a transaction on appDb
- stamps req.db, req.tenantId, req.tenantSlug
- awaits response finish before committing/rolling back so handler
queries land in the right transaction context.
Routes will switch from `db('Partner')...` to `req.db('Partner')...`
and add `tenantId: req.tenantId` to inserts. That refactor is the next
commit; this one just lays the wiring.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(tenancy): add tenantOf(req) helper for route handler ergonomics
* docs(multi-tenant): handoff guide for the in-progress refactor
Architecture decisions, what's committed, file-by-file refactor plan,
test fixup plan, and how to resume. Read this first before continuing
the multi-tenant work on this branch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(api): route + helper refactor for tenant-scoped req.db
Section A + B + C + E of the multi-tenant refactor: every route handler
now uses tenantOf(req) for a per-request transaction with app.tenant_id
pinned. Helpers (auth-sessions, auth.resolvePrincipal, config, mail-
settings, mailer, attribution, payouts, usage-billing, webhook-dispatcher)
take Knex + tenantId as parameters. tenantMiddleware is mounted in
app.ts; install + metrics stay public above it. Stripe webhook resolves
tenantId from event metadata and runs each event in appDb.transaction
with SET LOCAL app.tenant_id. Scheduler iterates active tenants per
tick. Typecheck passes.
What this leaves: section D (public /signup), F (test seed updates so
35 of 64 currently-failing tests go green), G (multi-tenant isolation
tests), H (env config + ops). Documented in docs/multi-tenant-refactor.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(api): public /signup + test seed tenantId fixes
Section D + F of the multi-tenant refactor.
D — POST /signup creates a Tenant + first Admin and emails an activation
magic link. Public, IP rate-limited (10/min), gated by slug validation
(/^[a-z0-9-]{3,30}$/, not in RESERVED_SLUGS, not already taken). Mounted
before tenantMiddleware in app.ts and uses the privileged db. Multi-mode
only — single-mode operators use /install.
F — every direct db().insert() in integration.test.ts, regressions.test.ts,
stripe-webhook.test.ts, and webhooks.test.ts now stamps tenantId:
DEFAULT_TENANT_ID. Test setups force OPENPARTNER_TENANCY=single. Cannot
verify against a live Postgres in this session; flagged as DONE BUT NOT
VALIDATED in docs/multi-tenant-refactor.md so the next pass runs the
suite first.
Handoff doc updated with current branch state and remaining work
(sections G, H + test validation).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(ops): multi-tenant env + docker + DO + docs
Section H of the multi-tenant refactor.
- .env.example: OPENPARTNER_TENANCY, OPENPARTNER_APP_DB_PASSWORD,
DATABASE_URL_APP with explanatory comments.
- docker-compose.yml: mount docker/initdb so postgres provisions the
openpartner_app role on first boot. Role is NOLOGIN if no password
set so RLS isolation can still be exercised via SET ROLE in tests.
- .do/app.yaml: add OPENPARTNER_TENANCY=multi (default for hosted),
DATABASE_URL_APP + OPENPARTNER_APP_DB_PASSWORD secrets on the api
component.
- docs/deploy-production.md: rows for the new secrets in the env
table; new "Multi-tenant rollout" subsection covering URL routing,
signup, RLS engagement, Stripe webhook tenant resolution, reserved
slugs, and the migration path from single-tenant.
The route, helper, signup, and stripe-webhook refactors plus this
ops layer make the multi-tenant branch deployable. What's left in
docs/multi-tenant-refactor.md is section G (live-Postgres isolation
tests) — needs a real DB to write meaningfully.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tenancy): bypass RLS on privileged db, commit trx pre-response, add isolation tests
Section G of the multi-tenant refactor + two real bugs the existing
suite surfaced once it ran against a real Postgres.
Bug 1: privileged db was subject to FORCE RLS. The migration role
owns the tenanted tables but FORCE RLS still gates the owner unless
row_security is explicitly off or app.tenant_id is set. Without
either, /metrics, /signup, the stripe-webhook tenant resolver, the
scheduler, and every test's direct cleanup query silently saw zero
rows. Fixed by adding bypassRls: true to createDb (sets row_security
= off in afterCreate) and turning it on for the privileged pool. The
appDb (tenant pool) keeps RLS engaged.
Bug 2: tenantMiddleware committed the per-request transaction on
res.on('finish'), which fires AFTER the response is sent. Tests doing
`await request(app).post(...)` then `await db(...).insert(...)` raced
the commit and got FK violations because the route's writes weren't
yet visible. Fixed by patching res.json/send/end so the trx commits
(or rolls back on 5xx) before any byte goes out. Belt-and-suspenders
res.on('close') still rolls back if the patched methods are bypassed.
Section G: apps/api/src/__tests__/multi-tenant.test.ts — 9 tests
that connect as openpartner_app via SET ROLE inside a privileged-pool
transaction (so RLS engages because openpartner_app has neither
BYPASSRLS nor superuser). Covers default deny, per-tenant visibility,
WITH CHECK rejection on cross-tenant inserts, platform_admin override,
session isolation, and the Tenant table self-policy. Suite skips
cleanly with a warning if the openpartner_app role isn't provisioned.
Stripe webhook tenant resolution: customer/invoice/charge events that
don't carry our metadata now fall back to a local Identity → Click
lookup so checkout-stitched customers still route to the right tenant
on subsequent invoice.paid / charge.refunded.
Result: 73/73 tests pass against the docker-compose postgres.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(network): creator self-signup + vendor↔Network protocol
Three pieces, designed so the same code paths cover hosted multi-tenant
and self-host:
1. Public POST /partner-signup (apps/api/src/routes/partner-signup.ts).
Tenant-scoped, IP rate-limited, creates a Partner row + magic link.
Honors a per-tenant partner_signup config (auto_approve vs
require_review, with disabled override). On hosted multi-tenant the
URL is /t/<slug>/partner-signup; on self-host it's /partner-signup.
2. Vendor-side Network client (apps/api/src/network-client.ts) +
NetworkOutbox migration. Fire-and-forget POSTs to /partners/upsert
on creator events (signup, admin invite, revoke); failures persist
to the outbox and the scheduler drains them every 5 min with
exponential backoff (~24h max). vendorToken stored AES-GCM
encrypted in Config (network_membership), never returned by GET
/config/network. backfillPartners(...) reconciles a vendor's
existing roster when they enable Network membership later — the
Network dedups on email and returns alreadyExisted=true for
creators who joined another vendor first.
3. Network protocol spec (docs/network-protocol.md). Defines the
/vendors/register, /partners/upsert, /vendors/backfill-partners,
and /vendors/me/heartbeat surface that openpartner-network
implements. Spells out the identity model (vendorId,
vendorPartnerId, networkCreatorId), auth rotation, and the
late-join reconciliation behavior.
Wired into existing flows:
- POST /partners (admin invite) + /partners/:id/revoke push to Network
when membership is enabled. autoEnroll gates new-partner upserts;
revokes mirror unconditionally so a Network-known creator stops
being matched after the vendor cuts them off.
- Settings router exposes GET/POST /config/network,
POST /config/network/backfill, and GET/POST /config/partner-signup.
- Scheduler runs network-outbox-drain every 5 min per active tenant.
Tests (apps/api/src/__tests__/network-and-signup.test.ts, 9 cases)
spin up an in-process HTTP receiver to act as the Network and verify:
signup without Network is silent; with Network on stamps
networkCreatorId on Partner.metadata.network; with Network down
enqueues outbox; drain retries succeed; require_review still pushes
status=pending; admin invite + revoke push; late-join backfill flips
preExisting=true for emails the Network already knew; GET
/config/network never leaks the vendor token.
82/82 tests pass against the docker-compose postgres.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(network): vendor-side onboarding integration
Wires the openpartner vendor side to the openpartner-network
self-serve onboarding flow.
network-client.ts: signupWithNetwork() POSTs to /vendors/signup;
completeNetworkConnect() POSTs to /vendors/verify-and-issue-token.
Failures surface immediately to the admin (no outbox queueing — a
failed signup is something the admin retries by hand).
routes/settings.ts: POST /config/network/start-connect mints a fresh
scoped key with NETWORK_FEDERATION_SCOPES, calls signupWithNetwork
with inferred instanceUrl + portalCallbackUrl, stashes partial state
in network_membership Config (enabled=false until verify lands).
POST /config/network/complete-connect consumes the magic-link ntoken,
calls Network /vendors/verify-and-issue-token, saves the returned
vendorToken with enabled=true. Same shape works for hosted multi-
tenant tenants (slug-aware URL inference) and self-host (request host).
routes/signup.ts: hosted multi-tenant signup auto-calls
signupWithNetwork after Tenant/Admin creation when NETWORK_URL env
is set. Best-effort: a Network outage doesn't fail the signup; the
admin can finish later via Settings → Network → Connect button.
Returns network: { status, vendorId } in the signup response so the
portal can show the right next-step UI.
.env.example: NETWORK_URL added with explanatory comment.
82/82 vendor-side tests still pass (no regressions; the new endpoints
are additive).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(portal): vendor admin Network UI — connect, offerings, requests
Closes the gap where vendors had backend wiring for Network membership
but no UI to use it. Without this, Network was invisible to vendor
admins on hosted multi-tenant + self-host.
Backend (apps/api):
- network-client.ts: NetworkProxyError + networkProxy.{listOfferings,
createOffering, updateOffering, deleteOffering, listRequests,
approveRequest, rejectRequest, whoami}. Decrypts the vendor token
from network_membership Config and proxies to Network endpoints
with the right bearer.
- routes/settings.ts: /admin/network/{me,offerings,offerings/:id,
requests,requests/:id/approve,requests/:id/reject}. Each is a thin
wrapper around networkProxy.* that turns NetworkProxyError into
the appropriate HTTP status. Required because the vendorToken is
a server-side secret — the portal can't hold it.
Portal (apps/portal):
- pages/admin/Network.tsx: connection status, contact-email/display-
name form for the Connect button, autoEnroll toggle, backfill
panel for late-join reconciliation.
- pages/admin/NetworkComplete.tsx: handles ?ntoken= callback from
the Network onboarding email; calls /config/network/complete-connect,
redirects to /admin/network on success. StrictMode-safe (one-shot
guard).
- pages/admin/NetworkOfferings.tsx: list + create + publish/unpublish
+ delete. Campaign dropdown pulls from /campaigns. Form fields:
title, description, productUrl, campaign, commission summary,
cookie window.
- pages/admin/NetworkRequests.tsx: pending requests list with creator
bio + pitch; approve dispatches federation (creates Partner +
Link on this instance); reject + status filter (pending /
approved / rejected / cancelled).
Wired into App.tsx routes + a new "Network" sidebar section
(Connection, Offerings, Requests).
Typecheck passes; portal builds (318 KB JS, 92 KB gzip).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Keith Fawcett <keith@brightyard.co>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the metering loop on the 3% Network fee. Previously the
openpartner main repo stamped Partner.metadata.network.creatorId
when a partner came through the Network, but never aggregated the
resulting payouts or surfaced them to anyone. The Network's billing
charged the flat $29 only.
usage-billing.ts: aggregateNetworkOriginatedPayouts(db, since, until)
sums Payout.amount where Payout.status='paid' AND
Partner.metadata.network.creatorId is not null AND completedAt is in
window (since, until]. Tenant scope provided by the caller.
network-client.ts: reportNetworkPayoutsToNetwork(db, tenantId)
- Loads network_membership; bails reason='network_not_configured' if
not enabled.
- Aggregates the period's total via the helper above, keyed off a
Config high-water mark (CONFIG_KEYS.LastNetworkPayoutsReportedAt).
- POSTs { amountUsd, sinceIso, untilIso } to the Network's
/vendors/me/report-payouts with the vendor token bearer.
- On 2xx, advances the high-water mark. On Network failure, leaves
the mark untouched so the next tick re-includes the same payouts;
the Network's identifier dedups at Stripe.
- On amount=0, advances the mark anyway (don't re-scan empty windows).
scheduler.ts: new cron 'network-payouts-report' at 03:30 UTC daily,
per active tenant via the existing forEachActiveTenant iterator.
Sits alongside usage-report (03:15) so they don't compete for the
same window.
config.ts: CONFIG_KEYS.LastNetworkPayoutsReportedAt added.
Tests (src/__tests__/network-payouts-report.test.ts, 8 cases) spin up
a local HTTP receiver acting as the Network endpoint. Cover:
- aggregateNetworkOriginatedPayouts:
- sums only paid payouts on Network-flagged Partners
- excludes pending; excludes payouts on direct partners
- respects (since, until] bounds
- reportNetworkPayoutsToNetwork:
- skips when membership not enabled
- zero amount: skip + advance mark
- happy path: right total, right bearer, mark advances
- Network 5xx: NO mark advance (so retry catches it)
- Subsequent run only includes new payouts (mark works)
90/90 vendor-side tests still pass.
Co-authored-by: Keith Fawcett <keith@brightyard.co>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the last gap in the Network billing flow: vendor admins now have a UI to subscribe / manage / view status. Previously they'd have needed to curl /api/vendors/me/billing/checkout directly. Backend (apps/api): - network-client.ts: extends networkProxy with getBilling, createCheckout, openPortal — same proxy pattern as the existing Network admin proxies (vendor token decrypted server-side, body forwarded through). Returns the typed Billing payload + Stripe URL responses. - routes/settings.ts: GET /admin/network/billing, POST /admin/network/billing/checkout (validates successUrl + cancelUrl), POST /admin/network/billing/portal (validates returnUrl). Each is a thin wrapper around networkProxy.* with the standard NetworkProxyError → HTTP status mapping. Portal (apps/portal): - pages/admin/NetworkBilling.tsx: branches on bundledWithMainPlan. Hosted vendors see a "bundled with Flex/Revshare" panel. Self-hosted see a status card (subscription state, current period end, $29 + 3% recap) plus EITHER a Subscribe panel (opens Stripe Checkout) or a Manage panel (opens Stripe Customer Portal). Subscribe flow surfaces specific errors when the Network deploy is missing STRIPE_SECRET_KEY / NETWORK_PRICE_ID / NETWORK_USAGE_PRICE_ID so the operator (and the vendor) know what's not configured. - App.tsx: route + new "Billing" nav item in the Network section (uses the existing CreditCard lucide icon). Tests (network-billing-proxy.test.ts, 9 cases): mocked HTTP receiver acting as the Network's billing endpoints. Covers the happy path for all three routes, body validation, missing-Network-config 503, admin auth gate, and Network 4xx/5xx surface as 400/503. 99/99 vendor-side tests pass; portal builds clean (322 KB JS, 93 KB gzip). Co-authored-by: Keith Fawcett <keith@brightyard.co> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tnerships, etc.)
Creators no longer have a separate Network portal — their home is the
vendor instance they signed up at. This adds the partner-side surfaces
that used to live at network.openpartner.dev directly into the vendor
portal so a creator on Vendor A can browse and apply to programs at
Vendors B/C/D, see all their cross-network partnerships, and edit a
single Network-wide profile.
API
- partnerProxy: server-side helpers that hit Network's existing
/offerings + /creators/me/* routes using the vendor's encrypted
vendorToken plus an x-act-as-vendor-partner header carrying the
partner's vendor-local id. Network resolves the Creator via
VendorAffiliation(vendorId, vendorPartnerId), so the same handlers
serve both browser-creator and proxied-from-vendor cases.
- /api/network/{discover,offerings/:id,vendors/:id} (open to anyone signed in)
- /api/network/me/{affiliations,affiliations/:id/earnings,requests,
requests/:id/cancel,profile} (partner-role only)
Portal
- pages/partner/{Discover,OfferingDetail,VendorDetail,MyAffiliations,
MyRequests,MyProfile} matching openpartner UI patterns (Page/Card/
Stat/StatusPill/Button).
- Sidebar gains a Network section for partners with Discover programs,
My partnerships, My applications, Network profile.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… strip
The multi-tenant build was 90% backend — `/install/status` correctly
returned `{ needsSetup: false, reason: 'multi_tenant' }`, but the
portal SPA had no routes to handle it (everything redirected to
/install) and the backend `tenantMiddleware` resolved tenant context
without stripping `/t/<slug>` from req.url, so downstream routers
mounted at root never matched. End result: app.openpartner.dev/install
loop, no way in.
Backend
- tenantMiddleware now strips /t/<slug> (and /api/t/<slug>) from
req.url after resolution, so the existing partnersRouter.get('/...')
registrations match cleanly.
- buildMagicLinkUrl(token, tenantSlug?) — sign-in / invite emails drop
recipients onto /t/<slug>/auth/magic so the SPA's tenant-aware api
client scopes the verify call. All 5 callers updated to pass slug.
Portal
- Multi-tenant routing in App.tsx based on install-status.reason: '/'
→ LandingPage, '/signup' → SignupPage, '/t/:slug/*' → Shell.
- api.ts derives tenant slug from window.location.pathname and
prepends /t/<slug> to every fetch — single-tenant unaffected.
- NavItem uses useTenantBase() so absolute paths like '/links' route
to '/t/<slug>/links' under multi-tenant. Sign-out + Shell login
redirect also tenant-scoped.
- Brand: real logo-mark-green.svg in Logo() (was a gradient 'O'
placeholder); favicons + apple-touch-icon + og-default wired in
index.html.
Operator: still needs to set OPENPARTNER_TENANCY=multi and
DATABASE_URL_APP on App Platform for the deploy to actually run in
multi mode with RLS enforced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Landing on app.openpartner.dev now offers two paths from day one:
- Vendors → /signup → create a tenant + admin account (existing)
- Creators → /creator/signup → create a Network Creator account
Creators get their own auth (magic-link via Network), their own Shell
at /creator/* with Discover, My partnerships, My applications, Profile.
A creator's identity is platform-wide; when they apply to a vendor's
program and the vendor approves, the existing federation flow creates
a Partner row in that vendor's tenant linked back to the same Creator.
API
- /api/creator-api/* reverse-proxies a fixed allowlist of Network
endpoints (creator auth, /creators/me/*, /offerings, /vendors/:id,
/offerings/:id/apply). Cookies pass through both directions, so the
Network's `op_network_creator_session` cookie ends up scoped to
app.openpartner.dev (Network sets it without a Domain, so the
browser scopes it to whoever responded).
- Mounted before tenantMiddleware so it works at the bare host with
no /t/<slug>/ prefix.
Portal
- pages/creator/{Discover,OfferingDetail,VendorDetail,MyAffiliations,
MyRequests,MyProfile,Signup,Signin,MagicLanding,Shell}
- creator-api.ts is a thin fetch helper for /api/creator-api/* with
cookie credentials and structured ApiError.
- Landing gains side-by-side Vendor + Creator CTA cards plus a
"Creator sign in" link in the header.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Brands is the term we want for the merchant side; "vendor"/"program" was internal language leaking into the UI. Three call sites: Landing CtaCard, Signup AuthFrame title, and the creator-signup cross-link. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Landing now mirrors openpartner.dev's tone — brand+creator parity, no
engineering jargon, the same headline ("Launch a partner program. Grow
it with the Network.") and the same value-prop bullets. Two equal-
weight audience cards driving to /signup and /creator/signup.
Sign-in is unified at /signin: one email field, no "are you a brand or
a creator" picker. Backend looks up Admin rows across all tenants and
the Network Creator by email; emails magic links for whichever match.
Always 200 silently to avoid email enumeration.
- POST /api/signin (multi-tenant only) — admin lookup across tenants +
Network creator forward, both best-effort.
- /signin SPA page with the standard "check your inbox" success state.
- Landing header: "Sign in" link replaces the creator-only signin
link; "Already have an account? Sign in" CTA below the audience
cards.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Modern browsers compile the HTML pattern attribute with the v flag, which rejects bare - between character class atoms. Both signup forms were using [a-z0-9-]/[a-zA-Z0-9_-] which the v engine flags as "Invalid character in character class". Escape the hyphen to keep the intended literal-dash semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related browser-side cookie bugs in the creator-portal proxy:
1. Node fetch comma-joins multiple Set-Cookie headers when you read
them via .get('set-cookie'), and the browser then can't parse the
resulting blob. Use getSetCookie() so each cookie ships as its own
header.
2. Cloudflare in front of network.openpartner.dev injects __cf_bm
with Domain=network.openpartner.dev. Relaying that to
app.openpartner.dev landed both Set-Cookie values mashed together
in one header, which confused the browser enough that the real
op_network_creator_session cookie wasn't stored at all (next
request only carried __cf_bm). Filter Cloudflare cookies out and
strip Domain= on whatever we do forward so it scopes to
app.openpartner.dev cleanly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Programs (Network offerings) are configured later from admin → Network → Offerings. Signup just provisions the brand's tenant + first admin. Adjust the labels accordingly: - Subtitle: "Create your brand account…" - "Program name" → "Brand name" - Submit: "Create program" → "Create account" - Success: "Program created." → "Account created." Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) * feat(review): brand approval gate + platform-ops console (anti-spam) New brands (a Brand is a Tenant) now land in a review queue instead of going live instantly, closing the self-serve spam/phishing vector. Data model (migrations + types): - Tenant.approvalStatus (pending|approved|rejected) + approvalReason, reviewedAt, reviewedByEmail. Backfills existing/self-host tenants to 'approved'; default stays 'approved' so only signup opts into 'pending'. - PlatformAdminSession, SignupBlocklist, PlatformAuditLog (platform-scoped, privileged-pool only, outside per-tenant RLS). Gating: - Router serves clicks only for approved brands (no attribution collected while pending). - approval-gate.ts 403s the go-live write actions (invite partner, creator self-signup, roster import, marketplace publish, creator-app approval) while pending; config + reads stay open so a brand can set up. Runs before the trial gate so a pending brand hears "under review", not "pick a plan". - Public signup + authenticated add-brand default to pending (add-brand auto-approves when the operator already runs an approved brand). Signup blocklist refuses banned emails/domains before a Tenant is created. Platform-ops console API (before tenantMiddleware, privileged pool): - Wires up the dormant PlatformAdmin table: magic-link operator auth (bootstrap via PLATFORM_ADMIN_EMAILS), sessions, requirePlatformAdmin. - Brand review: list queue, approve, reject (silent by default; optional brand notice + ban email/domain), reinstate. Blocklist + audit CRUD. Writes require role='admin'; 'support' is read-only. Notifications: ops "needs review" + decision emails to PLATFORM_OPS_EMAIL (platform transport); brand approved/rejected emails (rejection opt-in). Brand's own portal learns its state via approvalStatus on /branding. Tests: pure unit (route match + email parse) + DB-backed e2e (signup→ pending, blocklist, operator auth+verify, approve/reject+ban, gate lift, support read-only). Full suite 241/241 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(portal): platform-ops brand-review console + pending-review banner Adds the operator-facing UI for the brand approval feature (backend in the prior commit). All under apps/portal/src/pages/platform/, platform-level so it uses a raw papi() fetch helper (credentials:'include'), never the tenant-scoped api(). - /platform/login + /platform/auth: operator magic-link sign-in. - /platform/* PlatformConsole: guards on /platform-admin/me, top nav (Brands / Blocklist / Audit), operator identity + sign out. - Brands: pending/approved/rejected/all filter; approve, reject, remove (retroactive), reinstate. Reject/remove form defaults to a SILENT rejection with optional brand notice + ban email/domain, and copy that steers spam toward silent-reject-and-ban. - Blocklist + Audit pages. Read-only 'support' operators see lists without write controls. - Shell(): dismissible "under review" banner for a pending brand's own admin, reading approvalStatus off /branding via usePublicBrand(). Portal typechecks + builds clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#55) Brand-level info alone doesn't reveal a phishing/spam program — the tell is Program.destinationUrl (the cloaked/scam landing page). This surfaces each brand's programs to reviewers and adds a per-program takedown that leaves the brand on the platform. - Program.blockedAt / blockedReason / blockedByEmail (migration): reversible operator takedown, distinct from the scheduled endsAt wind-down. - Router: always resolves the Program (even for per-Link deep links) and 410s a blocked program's links — they stop redirecting immediately, independent of the brand's approvalStatus. - Ops API: GET /platform-admin/brands/:id/programs (destinationUrl + link count + block state); POST programs/:id/block {reason} + /unblock; audited. Brands list now carries programCount + blockedProgramCount. - Portal: expandable Programs section per brand card showing the destination as plain text (never a live link) with admin-only Block/ Unblock. Tests: list + block/unblock + support-read-only. Full API suite 243/243. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rejecting a brand or blocking a program took it dark LOCALLY (tenant suspended, links stop redirecting) but left its listing live on the Network marketplace, still taking creator applications. A rejected phishing brand stayed publicly listed. Offerings live on the Network — a separate service. The Network already filters `Offering.published = true AND Vendor.status = 'active'` on both the marketplace list and the offering detail page; it was simply never told. Nothing in the reject/block path crossed the federation boundary. - network-client: adminSuspendVendor / adminReactivateVendor, authed with NETWORK_ADMIN_API_KEY (mirrors adminRestoreVendor). - rejectBrand → suspends the brand's Vendor on the Network, which pulls every offering it published off the marketplace at once (and kills its vendorToken). approveBrand/reinstate → reactivates it. - blockProgram → unpublishes that program's offering (published=false); unblockProgram → re-lists it only if the brand had shareOnNetwork set. All propagation is best-effort: a Network outage must not block a local takedown. Failures are logged and re-running the action retries. Tests: reject → asserts POST /admin/vendors/:id/suspend (with the admin bearer + reason) and reinstate → /reactivate; program block → asserts the offering is PATCHed published=false. Full API suite 245/245. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rm (#57) Completes the partner-moderation half. Creators live on the Network (one identity across every brand), so blocking is a NETWORK-level act — distinct from a brand revoking a partner from its own roster (POST /partners/:id/revoke). These routes proxy the Network's ADMIN_API_KEY-gated /admin/creators API. - network-client: adminListCreators / adminBlockCreator / adminUnblockCreator (callNetworkAdmin generalized to GET+POST); platformNetworkUrl() since creator moderation is cross-tenant and uses the env origin, not a brand's membership Config. - platform-admin: GET /platform-admin/creators, POST /creators/:id/block {reason} + /unblock. Audited. Writes require role='admin'. 503 when NETWORK_URL is unset, 502 (with detail) when the Network call fails. - Portal: /platform Creators tab — All / Incomplete / Blocked filters, avatar + handle + brands/platforms counts, the "hidden from discovery" flag with the exact missing fields (from the Network's completeness gate), and admin-only Block/Unblock. Tests: list + block + unblock assert the Network admin calls fire with the admin bearer and the operator's email; support operators are read-only. Full API suite 247/247; portal typechecks + builds. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Node 20 reached end-of-life in March 2026 and no longer gets security patches. Bump the runtime floor to Node 22 (current LTS) across every pin: Docker build args (api/portal/router), .nvmrc, root engines (>=22), and the CI + npm-publish workflow node-version. No dependency or lockfile change; no source change. Surfaced by Codex during the SSRF hardening review. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
PartnerProgram (formerly PartnerCampaign) was created after the global RLS/grant migrations and never got its own tenant_isolation policy or openpartner_app DML grant — every other tenant table created since (PartnerPostback, etc.) does. The rename migration only assumed a policy existed. Prod runs the app role (DATABASE_URL_APP + OPENPARTNER_APP_DB_PASSWORD set, OPENPARTNER_TENANCY=multi), so PartnerProgram queries would fail permission-denied when the partner↔program feature is exercised; on a privileged-pool deploy the absent policy means no tenant filter. Adds enable/force RLS + tenant_isolation policy + idempotent app-role grant, mirroring 20260613000000_partner_postback.ts. Verified locally: relrowsecurity + relforcerowsecurity = t, policy present (ALL, USING+WITH CHECK), grant present. NOTE: prod migrations are manual — run `pnpm migrate` against prod after merge. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…gap) (#61) /coupons/redeem was gated only by requireAuth + grantScope('events:write'). grantScope is a pass-through for any non-scoped principal, so a logged-in PARTNER (session or partner API key) reached the handler and could forge conversions — synthesizing Click+Identity+Event and minting commissions crediting themselves. /clicks had the same shape (grantScope('clicks:write') with no role gate); lower impact since the handler derives partner/program from the Link row, but it's still a server-to-server ingest route that must not accept partner principals. Both now match the sibling /attribution/events route: requireAuth → grantScope → requireAdmin. Scoped federation keys (clicks:write / events:write) still work — grantScope rewrites them to admin before requireAdmin runs. Added regression tests asserting a partner key gets 403 while admin/scoped reaches the handler. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…64) * feat(security): SSRF guard for outbound webhooks + partner postbacks Both server-side fetch sinks took attacker-influenced URLs with only syntactic (z.string().url()) validation: webhook endpoint URLs (tenant admin) and partner postback URLs (partner — lower privilege), then fetched them with no host/IP restriction and default redirect-following. Neither is fully blind — the webhook test route returns status/error and postbacks store lastStatus/lastError — so both were usable as internal port-scan / metadata oracles (169.254.169.254, RFC1918, loopback, …). Adds a shared block-by-default guard (outbound-guard.ts) applied at both sinks: - http/https only, no embedded credentials, port on the policy allowlist. - Resolve EVERY address; reject unless each is globally-routable public unicast (ipaddr.js). Rejects IPv4-mapped IPv6, and octal/hex/decimal IPv4 encodings (WHATWG URL normalizes them before classification). Special-use hostnames (localhost/.local/.internal/.home.arpa/single-label) refused. - DNS-pin the validated address into a per-request undici dispatcher's connect.lookup so DNS can't rebind between check and connect; the original URL still supplies Host/SNI/cert identity. - redirect: 'manual', zero hops — a 3xx is a non-delivery. Blocked destinations throw OutboundBlockedError with a stable code (safe to store in partner-visible lastError), recorded as a failed delivery. Policy is deployment-scoped: hosted locks ports to 80/443; selfhost+single leaves ports open and honors OPENPARTNER_OUTBOUND_ALLOW_PRIVATE_CIDRS (an escape hatch that FAILS STARTUP if set in any other mode). Checked prod first: all 9 configured webhooks are public HTTPS/443 and there are 0 postbacks, so block-by-default breaks nothing live. Design reviewed adversarially with Codex. Deps: undici + ipaddr.js. Tests: outbound-guard.test.ts (schemes/ports/creds/IP classes/encodings/hostnames/ escape hatch + a live-server check that a blocked host never gets a socket); existing webhook + postback integration tests set the selfhost escape hatch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(security): address SSRF review — partner postbacks public-only, fail-fast boot Two findings from Codex review of the SSRF guard: 1. (HIGH-when-enabled) The private-CIDR escape hatch was shared with partner-controlled postbacks, so an operator opening an internal range for an admin webhook also handed every partner a scan/request oracle across it. safeFetch now takes a trust level: partner postbacks use a hardened policy (escape hatch stripped, ports never unrestricted — 80/443 fallback); admin webhooks keep the full deployment policy. Callers pass trust: 'admin' (webhook-dispatcher) / 'partner' (partner-postback). 2. (LOW) "Refuses to boot on bad config" was documented but not enforced — the policy built lazily on first delivery. server.ts now calls outboundPolicy() before listen(), so a misconfigured OPENPARTNER_OUTBOUND_* fails startup. Tests: new guard case asserting admin honors the escape hatch while partner does not (and never opens a socket); the postback integration test uses the new __setOutboundPolicyForTests override for its localhost receiver (the security split itself is covered in outbound-guard.test.ts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(billing): make usage reporting exactly-once (outbox)
The Stripe meter-event identifier was tied to a per-run rangeEnd (new Date()),
while the high-water mark only advances after Stripe accepts. A crash (or
timeout) after Stripe accepted but before the mark advanced meant the next
run aggregated an OVERLAPPING window with a NEW identifier — double-billing
the tenant for the same GMV.
Freeze {rangeStart, rangeEnd, amount, identifier} to a pending Config row
BEFORE the Stripe call; clear it only after the mark advances. On the next
run, a surviving pending row is re-sent VERBATIM (same identifier ⇒ Stripe's
Meter Events API dedupes) rather than re-aggregating. No schema change
(uses the per-tenant Config table); added deleteConfig helper.
Tests: a clean run freezes-then-clears; a simulated crash after the meter
call leaves the pending row, and the retry re-sends the SAME identifier +
amount (ignoring GMV accrued in between), proving exactly-once.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(billing): usage reporting — concurrency dedupe + stale-pending guard
Two review findings on the exactly-once outbox:
- (concurrency) The meter-event identifier was keyed on a per-run rangeEnd,
so two runs racing the same period (scheduler + manual /billing/report-usage,
or two replicas) each froze a different rangeEnd/identifier and submitted the
same GMV twice. Key the identifier on the period START (the high-water mark)
instead — identical for concurrent same-period runs, so Stripe dedupes;
distinct periods still get distinct keys.
- (stale pending) A pending row left by a multi-week outage could never be
resent (Stripe rejects meter events past its ~35-day timestamp window),
wedging the tenant's reporting forever, and past Stripe's ≥24h dedup window a
resend risks double-billing. If the frozen rangeEnd is >30d old, abandon it:
advance the mark and ALERT for manual reconciliation (a rare, loud
under-report beats a silent double-charge or a permanent wedge).
Tests: identifier carries the period-start suffix (not rangeEnd); a 40-day-old
pending row is abandoned (no Stripe call, mark advanced, pending cleared).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…uard, nightly Stripe reconcile (#60) * fix(billing): catch tenant cancellations — ops alerts, end-customer guard, nightly Stripe reconcile A hosted tenant cancelled via the Stripe Customer Portal on Jul 17 and we never noticed: the prod webhook endpoint wasn't subscribed to customer.subscription.updated/deleted (docs listed neither), so the local billing mirror, white-label entitlement, and custom-domain routing all stayed live, and nobody was emailed. Three fixes: - Ops notifications (PLATFORM_OPS_EMAIL) on tenant billing lifecycle: cancellation scheduled/resumed (detected via previous_attributes on subscription.updated), subscription ended, and dunning failures on the tenant's own invoices. Shared sendOpsEmail extracted from brand-review. - Guard the webhook's tenant-billing paths on the event's customer being the tenant's OWN billing customer. Previously a merchant end-customer's subscription.updated/deleted (resolved via the Identity chain) would clobber Tenant.stripeSubscriptionId and could disable white-label. - Nightly billing-subscription-reconcile job (04:25 UTC): polls Stripe for every tenant that locally claims a subscription and heals missed-webhook drift — ended subs clear the mirror + revoke white-label/custom-domain exactly like the deleted webhook, live subs refresh HostedBillingState, newly discovered cancel_at_period_end alerts once (Config-keyed dedupe). Docs: deploy-production.md now lists subscription.updated/deleted as required events for Destination A and explains why they're load-bearing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(billing): harden the cancellation-sync paths per adversarial review Codex review of PR #60 surfaced five real gaps; all fixed: - resource_missing NEVER heals: a canceled sub stays retrievable forever, so "missing" means the key can't see the id (wrong account / test-mode key). Healing on it would let one config mistake mass-revoke every live tenant's white-label + custom domain. Now reported as an error instead. - Stale-subscription safety: the deleted webhook and the nightly reconcile clear the pointer with ONE conditional UPDATE keyed on the exact sub id the event/poll referred to — a late retry for a since-replaced sub, or a resubscribe racing the poll, matches 0 rows and is skipped. The updated webhook likewise ignores subs that aren't the tenant's current one and never adopts a terminal sub into an empty pointer. Own-billing detection gains a sub-id fallback for tenants with no persisted stripeCustomerId. - Webhook-retry email dedupe: the cancel-notice Config marker now guards the webhook path too (not just the reconcile), deleted retries are silenced by the conditional clear, and dunning alerts dedupe per (invoice, attempt) so redeliveries stay quiet while every new collection attempt still notifies. - cancel_at coverage: cancellations scheduled via a bare cancel_at (dashboard date / Subscription Schedules) now notify, not just the Customer Portal's cancel_at_period_end. - Notify-then-mark: sendOpsEmail reports transport success and the cancel-notice marker is only written after a successful send, so an SMTP outage retries next pass instead of losing the notice forever. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(billing): close null-pointer resurrection in the subscription.updated guard Open review found a hole in the PR #60 stale-sub guard: it only rejected TERMINAL snapshots when the pointer was null, so a late/resent/out-of-order ACTIVE subscription.updated arriving after subscription.deleted (pointer already cleared) passed both guards and re-persisted the old sub id, resurrecting the canceled subscription and re-applying white-label. Stripe guarantees no event ordering, so this reordering is real. An updated may now touch billing state only when it refers to the tenant's CURRENT subscription (pointer non-null AND equal to sub.id). A null pointer is re-established solely by checkout.session.completed on a real (re)subscribe, so an updated never legitimately adopts one from null. Added a test for the active-after-delete ordering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…in prod (#63) * fix(commissions): repair auto-approve SQL — was failing for every tenant in prod The nightly commission-auto-approve job has been throwing in prod for all tenants: `invalid reference to FROM-clause entry for table "c"`. The UPDATE...FROM referenced the update target `c` inside a FROM-clause LEFT JOIN (`pc."partnerId" = c."partnerId"`), which Postgres forbids. Net effect: matured accrued commissions never auto-approved, so they never progressed to payout — platform-wide, every night. Fix: key the PartnerCommission join on `a."partnerId"` (Attribution is already a legal FROM entry; a commission's partnerId equals its attribution's partnerId by construction, and c."attributionId" = a.id pairs them). No behavior change beyond the query now executing. Adds commission-auto-approve.test.ts (5 cases incl. partner-snapshot-holdback preference) that runs the real query against Postgres — there was no test exercising this SQL, which is how it reached prod. Discovered while checking prod logs (doctl) for an unrelated RLS finding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(commissions): defensive partnerId match in auto-approve join Per review: also require c."partnerId" = a."partnerId" in the auto-approve UPDATE. It's equal by construction (a commission's partner comes from its attribution), but the DB doesn't constrain it, so this guards against any future data-integrity drift approving a mismatched row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…out guard) (#71) reconcileIntent listed only the first 100 transfers in a batch's transfer_group. A batch with >100 transfers whose match sits on a later page looked "absent", so the intent was reset to pending and RE-POSTED — a duplicate partner transfer. Follow has_more to exhaustion (stop early on match). Isolated, Codex-tagged safe subfix of the funding-race set (#12); the flag stays off. Test: an ambiguous intent whose landed transfer is on page 2 now reconciles (confirmed, no re-POST) and the listing pages twice. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix: four safe audit findings — entitlement, attribution, storage Mechanical, low-risk fixes from the audit (Codex-verified as safe-to-fix): - Enterprise self-assign (billing.ts): POST /billing/plan accepted plan='enterprise', which counts as active + enables white-label with no Stripe subscription. Restrict the setter schema to flex/revshare (signup already excludes enterprise). - Delinquent keeps white-label (white-label.ts): isWhiteLabelEntitled keyed only on a non-null subscription id, so an unpaid/paused/canceled sub kept white-label + custom domain live for free. Route it through hasActivePlan, which already rejects delinquent statuses (past_due still entitled — Stripe is dunning). - Future-click attribution (attribution.ts): the window check only rejected ages > window, so a click that happened AFTER the event (negative age) could attribute an earlier conversion during backlog/backdated processing. Reject ageMs < 0 too. - Upload ENOENT (storage.ts): the fs backend created only the storage root, so a nested key (tenants/<id>/logos/...) failed on a fresh self-host install. mkdir the full parent path. Tests: delinquent-status white-label cases, a nested-key storage write, the enterprise-rejection route check, and a negative-age attribution case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(attribution): give the negative-age check a clock-skew grace CI on this branch was red — 7 tests across compound-rules.integration and stripe-webhook. Two different causes hid behind one symptom. The real one: `ageMs < 0` is too strict to be correct. Event.ts comes from `new Date(event.created * 1000)` and Stripe stamps `created` in whole SECONDS, so a conversion in the same second as its click truncates to up to 999ms BEFORE it. The clock stamping Click.ts is also not the clock stamping the event. Both cases are ordinary, and a strict check dropped the attribution silently — no error, just a partner not getting paid. Rejecting a later click attributing an earlier conversion is still right; that case is hours or months off, so a 5-minute grace separates them cleanly. The other: the compound-rules fixtures seeded the click at NOW against events backdated to January. That is exactly what the guard targets, so they were correctly rejected. Fixed the fixtures to put the click just before the first event — which also means the 60d window is genuinely exercised there for the first time, since the negative age used to short-circuit the check. Added the companion to #66's own negative-age test: an event 2s before its click must still attribute. Verified it fails when the grace is reverted. Full API suite green (37 files / 256 tests), typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Metering counted only invoice_paid + subscription_created toward attributed GMV, so a merchant reporting revenue under a CUSTOM event type accrued partner commissions but escaped the platform % (revshare 3% / flex 1.5%). Make the billable set an explicit, documented semantic: default to the two Stripe- native types, extend via OPENPARTNER_BILLABLE_EVENT_TYPES_EXTRA (comma- separated) for operator-known custom revenue types. aggregateAttributedGmv now takes the event-type set (default = configured) so it's explicit at the call site and testable. Chosen (with the user) as the safe option: it changes NO existing invoice and locks in no possibly-wrong auto-semantic — no GMV definition is fully dodge-proof on self-reported data (percent-base just moves the dodge to fixed commissions; count-all over-bills mislabeled values). Per-tenant granularity + auto-detection are follow-ups. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
#67) * fix(refunds): stopgap — only auto-reverse commissions on a FULL refund charge.refunded reversed 100% of an invoice's accrued/approved commissions on ANY refund, and charge.amount_refunded is cumulative — so a $1 refund on a $100 order wiped the entire commission, and successive partials mis-stated the reversal. Codex-recommended safe stopgap: only auto-reverse when the refund is full (amount_refunded >= amount). Partial refunds are still recorded as a corrective 'refund' event and flagged with partialRefundReversalSkipped for manual handling. This is a stopgap, not the end state: proportional clawback (incremental refund deltas + immutable CommissionAdjustment rows) is the real fix and a separate ledger change. The stopgap strictly prevents the current partner-harming over-clawback. Tests: existing full-refund cases now carry charge.amount (so they remain full and still reverse); new case asserts a partial refund leaves commissions accrued and sets the skip flag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(refunds): measure "full refund" against amount_captured, not amount The stopgap compared amount_refunded to charge.amount (the INTENDED amount), but Stripe's amount_captured is what was actually collected and can be smaller after a partial capture. A $100 auth captured for $60 then fully refunded ($60) has amount_refunded=6000 < amount=10000, so it was wrongly classified partial → commissions left payable on a fully-refunded sale. Base the full-refund test on amount_captured (fallback to amount when absent). Test: a partially-captured ($60 of $100) charge, fully refunded, now reverses commissions (fullRefund=true). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…fused-deputy) (#68) * fix(security): bind Stripe webhook secrets to their event family (confused-deputy) /webhooks/stripe verified against any secret in one combined STRIPE_WEBHOOK_SECRET and trusted openpartner_tenant_id metadata to establish the tenant — so a holder of ANY configured secret could forge an event from the OTHER destination (e.g. a Connect secret carrying a fabricated checkout.session.completed / invoice.paid with attacker-set tenant metadata) and mint subscriptions, commissions, or funding state for any tenant. Not outsider-exploitable today (no merchant holds a verifying secret), but a latent cross-tenant confused-deputy. Split the signing secrets by Stripe "Event destination": - STRIPE_WEBHOOK_SECRET_PLATFORM → platform-account events (checkout.*, customer.*, invoice.*, charge.*, payment_intent.*) - STRIPE_WEBHOOK_SECRET_CONNECT → connected-account events (account.updated, transfer.*) Verification records which family the matching secret belongs to; an event whose type doesn't match its verifying family is rejected (secret_family_mismatch) before any funding/tenant processing. Connect events now resolve the tenant from the connected-account id (authoritative), not attacker-influenceable object metadata. Back-compat: the legacy combined STRIPE_WEBHOOK_SECRET still works (verifies both families, enforcement OFF) with a startup deprecation warning, so existing prod deploys are unchanged until they adopt the split vars. Docs + .env.example updated with the migration. Note: securely supporting merchant-pointed ("Rewardful") webhooks would need a THIRD, per-merchant secret family scoped to attribution-only events — out of scope here and still unsupported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(security): close two review findings in the webhook-trust change Codex review of the branch found: 1. (CRITICAL) account.updated selected the partner to update from account.metadata.openpartner_partner_id — a Standard account can influence its own metadata, so partner A setting it to B's id would re-point B's stripeConnectAccountId and hijack B's payouts. The partner↔account link is always established server-side at /connect/start (accounts.create → persist the id), so resolve the target (and the tenant) by the connected-account id ONLY and drop the metadata path entirely. 2. (HIGH) transfer.updated/transfer.reversed were classified as connected-account (Connect) events, but we create Connect transfers with the PLATFORM key, so Stripe fires those on the platform account — they arrive on Destination A. As connect-family they'd be rejected secret_family_mismatch (breaking funding reversal handling) once the split secrets are adopted. Moved them to PLATFORM_EVENT_TYPES; only account.updated remains connect. Deploy doc updated (transfer.* under Destination A). Tests: forged-metadata account.updated leaves the victim's account untouched; transfer.reversed passes on the platform secret and is rejected on the connect secret. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…#12) (#75) * fix(funding): close the three pipeline races + a live-Stripe backstop (#12) HOSTED_FUNDING_ENABLED is OFF, so all of this is latent — but every one of these costs the brand real money the day the flag flips, and a money state machine is not something to patch piecemeal. This is the coherent pass the handoff asked for, with the staging matrix extended to match. 1. AMBIGUOUS PaymentIntent CREATE re-created instead of searching. A create whose response was lost (timeout, crash) left the batch funding_failed with no PI stamped — even though Stripe may well have made a real ACH debit. Inside Stripe's ~24h idempotency window the frozen key `fbpi:<batchId>` replays harmlessly; PAST it the retry created a SECOND PaymentIntent, i.e. debited the brand twice. Now: once a batch has attempted at all, the retry SEARCHES by our metadata stamp and adopts what it finds (routing on the PI's actual status), and only creates when Stripe confirms there is nothing. Also, a create that fails *with* an intent attached (declines) now records that id, so the retry confirms it instead of making another. 2. RELEASE vs IN-FLIGHT CREATE. The PI id is only stamped when create returns. A release landing in that window saw no PI, so it skipped terminalization and freed the allocations — collecting money for commissions already back in the pool. Fixed from both sides: - the stamp is now status-predicated; losing that CAS means a release took the batch, so the orphaned PI is canceled. If Stripe won't cancel it (already processing), the batch is frozen recovery_required with a loud alert rather than left silent. - release no longer trusts "no id on the row": it asks Stripe before freeing anything, and a search that FAILS returns pi_not_terminal. Don't know ⇒ don't free. 3. INBOX CLAIMED BEFORE PROCESSING. The inbox row was written before the handler ran, so a crash mid- handler made every Stripe redelivery a no-op — the transition it carried was lost permanently. The claim is now a LEASE: only a stamped outcome is terminal, an unfinished claim older than 5 minutes can be taken over by a redelivery, and handlers are CAS-based so a takeover racing a live worker degrades to a lost CAS. A claim still unfinished an hour later (crashed AND never redelivered) is alerted by the daily reconcile. Plus the gap the handoff flagged for confirmation: refunds, disputes and transfer reversals had NO live-Stripe backstop — the reconcile only looked at locally-flagged state, which by definition cannot see a lost webhook, so a missed reversal left a payout recorded paid and its batch unfrozen. The daily job now sweeps settled money against Stripe (bounded at 50/run, and it LOGS what it didn't reach rather than looking clean): a refunded or disputed funding charge freezes the batch exactly as the webhook would, and a reversed transfer is recorded through the same reversal handler. 19 DB-backed tests with a Stripe mock cover all four, including the two that can only be seen by interleaving — a release landing mid-create, and a redelivery arriving after a claim's worker died. Runbook: docs/payout-funding-staging-runbook.md gains section H with the eight staging scenarios that prove the Stripe half of these (how to force each one), a G4 row for the stuck-claim alert, and the four invariants worth re-reading before anyone touches this pipeline again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): adversarial-review fixes — the inbox fix was incomplete, plus six more Codex review of #75 refuted all four claims. The most important finding is that my own inbox fix only moved the failure window rather than closing it. 1. THE INBOX STILL LOST EVENTS. `claimInboxEvent` returned a bare boolean, so "already finished" and "another worker holds it" both became `inbox_replay` — and the route answered 2xx. A worker that crashed mid-handler therefore lost its event whenever Stripe's redelivery arrived INSIDE the five-minute lease: the second worker acked it, and Stripe never delivered again. Lost forever became lost on a 5-minute fuse. The claim now reports claimed/done/held, `held` raises InboxEventHeldError, and the webhook route answers 409 so Stripe redelivers after the lease expires. 2. THE LEASE HAD NO OWNER. A worker whose lease had been taken over could still stamp an outcome onto — or delete — the new owner's claim, since both keyed on event id alone. The claim's `processedAt` now doubles as an owner token: a takeover swaps it, and stamp/release are scoped to the token they were issued. 3. THE REVERSAL SWEEP SKIPPED `partially_reversed` FOREVER. Once a partial reversal was recorded, the webhook completing it could be lost and the sweep — the only backstop — never looked again. Only a fully `reversed` payout is finished. 4. BOTH SWEEP CAPS STARVED. Taking the oldest 50 of an ever-growing list meant the same 50 were re-checked nightly while newer batches — the only ones that can still change — were never checked at all. Now bounded by the 180-day reversal horizon (everything inside it is checked) and anything past the cap is REPORTED by id, not just counted. 5. AN ORPHANED PI LEFT ITS ALLOCATIONS FREED. When a release won the race and the created PI couldn't be canceled, the batch froze recovery_required but its allocations stayed `released` — so the very same commissions could be re-batched and the brand charged twice. The orphan path now reclaims allocations no newer batch has taken. 6. `release_requested` WAS TERMINAL BY ACCIDENT. A release that stopped on a Stripe failure returned 'pi_not_terminal', but no collector state matched that status and a second releaseBatch call just lost the CAS — so the batch sat forever with its allocations frozen, unalerted. It is now a re-entrant source state, the collector resumes it every tick, and the daily reconcile alerts on one stuck over a day. 7. PAYMENT-WINS SKIPPED VERIFICATION. Release CASed straight to `funded` without stripeChargeId/fundedAt, so the executor froze the batch as recovery_required on the next tick. It now goes through confirmFundingFromPaymentIntent — the one verified transition — and freezes explicitly if verification refuses. Also: the executor re-reads the batch status before each partner transfer, so a dispute freezing the batch mid-run actually stops it; and the CommissionAdjustment clawback insert is serialized under a row lock, because check-then-insert with no unique constraint could double-record when a redelivery and the sweep run concurrently. 12 new/updated tests. Runbook gains H9–H12 and the corrected invariants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): round-2 review — orphan reclaim was unreachable, plus four more Second adversarial pass over the fixes. The headline finding is that the round-1 orphan recovery could not run at all in the case it was written for. 1. BLOCKER — RECLAIM SAT BEHIND A CALL THAT RAISES. abandonOrphanPaymentIntent escalated the batch to recovery_required BEFORE reclaiming its allocations. But `funding_disputed`/`recovery_required` are inside the "one open batch per tenant/currency" unique index, so moving a released batch back into it violates the index whenever a newer batch is already open — and casBatch raises rather than returning null. The throw skipped the reclaim entirely, leaving a live ACH debit AND freed commissions, which is the exact double-charge the path exists to prevent. Reclaim now runs first and unconditionally; the escalation is best-effort, and the alert names how many allocations a newer batch had already taken. 2. THE SAME INDEX BROKE REFUND/DISPUTE HANDLING. `charge.refunded` on a settled batch tried to drag it back to funding_disputed and threw the webhook (and the sweep) with it. A settled batch has already transferred, so freezing buys nothing: non-terminal batches are frozen, terminal ones record the clawback and alert. Both the webhook and the reconcile sweep now share one function so they cannot drift. 3. THE SWEEP STILL STARVED. Reporting skipped ids made the cap visible but didn't change the scheduling — a fixed order plus a fixed prefix re-checked the same head every night. Rows are now dealt by a per-day hash, so a capped run covers a different slice each day and the whole set over time. 4. THE 180-DAY HORIZON WAS WRONG FOR TRANSFERS. Refunds have a deadline; transfer reversals do not — Stripe places no age limit on them. The horizon now applies only to the charge sweep; the transfer sweep covers every confirmed intent, with rotation making that affordable. 5. RESUMING A RELEASE RESET ITS OWN ALERT CLOCK. casBatch bumps updatedAt and reconcile decides "stuck" from updatedAt, so a release failing every five-minute tick refreshed the 24h alert forever and stayed silent. Re-entry no longer rewrites the row. Also: transfer reversals are paged rather than trusting the ten embedded in the Transfer object — under-counting them is what decides partially_reversed vs reversed, and therefore whether the clawback adjustments get written at all. 5 new tests, including the blocker with a competing newer batch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(funding): inject the Stripe client into the reversal pager The pager I added in f0f7578 reached for the global `requireStripe()` instead of the client its caller already holds, and carried a `db` parameter it never used. In tests that means a paged reversal would have bypassed the injected mock and hit the real client; in production it happened to be the same object, which is exactly the kind of coincidence that stops being true later. `handleTransferReversed` now takes the client explicitly, like every other Stripe-touching function in this module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): round-3 review — the rotation didn't cover, and a clawback could be outrun Third adversarial pass. The headline is that round-2's "rotation" fix gave the appearance of coverage without the substance. 1. THE SWEEP STILL DIDN'T COVER EVERYTHING. The per-day hash shuffle re-deals independently every day, so it is not a rotation at all: at 10k rows and 50/run, an individual row had roughly a 40% chance of never being checked before ageing out of the horizon. Replaced with a sliding WINDOW over a stable ordering — every row is visited within ceil(total/limit) days, by construction, with the day number acting as the cursor so concurrent workers pick the same slice. The old test ("two days differ") passed for the broken version too; the new one runs a full cycle and asserts every row was checked. Also: `now()` was called per use, so a run spanning a UTC midnight could straddle two windows. Captured once. 2. AN OUT-OF-ORDER CLAWBACK COULD BE ACKED, THEN THE BATCH PAID. The freeze only accepted payment_processing/funded/transferring. Stripe doesn't order webhooks, so a refund arriving while the batch was `release_requested` recorded nothing — and confirm.ts explicitly allows release_requested → funded, after which the executor pays normally. The freeze now accepts every non-terminal state. 3. A TRUNCATED REVERSAL LEDGER WROTE TERMINAL STATE. The 20-page cap returned an incomplete list, and payout status was derived from it — a fully-reversed transfer could sit at `partially_reversed` forever, with every later run re-reading the same truncated prefix. The pager now reports completeness and an incomplete read derives NOTHING; it alerts for operator reconciliation instead. 4. THE ORPHAN-RECLAIM STUCK STATE WAS INVISIBLE. Reclaiming allocations back to `reserved` under a batch that then fails to re-open leaves a live allocation owned by a terminal batch: reservation won't re-take the commission (live-allocation index) and the invariant loop skips released batches, so the partner simply never got paid and nothing said so. Reconciliation now scans for live allocations under terminal batches and alerts. 5. THE EXECUTOR'S FREEZE CHECK WAS A READ, NOT A GATE. The per-partner status re-read added in round 2 is a check; a dispute landing between it and the transfer still got paid. The batch status is now verified with FOR UPDATE inside the intent-creation transaction, so a frozen batch cannot have a new intent committed. (The round-1 test was vacuous — it froze the batch before invoking the executor, so the query never selected it. Rewritten to freeze mid-run.) 6. A clawback on an already-settled batch was recorded on the row but not returned by /billing/funding, so the portal rendered it as a healthy "Settled". Surfaced in the API and flagged in the UI. 4 new tests plus two rewritten ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): round-4 review — a real cursor, and the executor gate stopped short Fourth adversarial pass. My sweep-coverage fix failed for the SECOND time, in a new way, and the round-3 executor gate didn't cover the case it was written for. 1. HIGH — THE SWEEP WINDOW STARVED UNDER CHURN. `(day % ceil(total/limit))` only guarantees coverage for a FROZEN list. Production adds rows daily, so `windows` changes, the modular sequence shifts, and specific indices are never selected — the review produced a concrete 10k-row schedule where a target row is never visited in 180 days and then ages out of the horizon. Replaced with a persisted CURSOR: it walks ids in order and remembers where it stopped, so churn cannot starve anything and rows added behind it are picked up on the wrap. (Two failed attempts at this — a hash shuffle, then a count-derived window — both looked like rotation without guaranteeing coverage. A cursor is the boring answer.) 2. HIGH — THE EXECUTOR GATE ENDED BEFORE THE TRANSFER. Round 3 put the batch check inside the intent-creation transaction, which only runs for intents created on that pass: an existing pending/failed/posted intent skipped it entirely, and even a new one could have its batch frozen between that commit and the POST. There is now a final check immediately before `transfers.create`. It cannot be atomic with a Stripe call, but it closes the existing-intent hole and narrows the rest to the request itself. 3. HIGH — THE TRUNCATION GUARD THREW AWAY GOOD RECORDS. Returning early on an incomplete reversal list also discarded the reversals it had already fetched — a valid audit trail — and re-fetched the same prefix forever. Those rows are now recorded (the unique stripeReversalId makes it idempotent); only the DERIVATION of payout status is withheld. 4. LOW — `failureReason` was returned raw to a brand-facing admin UI. It carries raw Stripe error text, internal state-machine reasons and Stripe object ids (`orphan_payment_intent:pi_…`). The API now returns a normalized `needsAttention` flag; operators get detail from the logs. 5. LOW — the orphan-allocation scan pushed one attention id per ALLOCATION, so a batch with 100 of them appeared 100 times. Deduped. Three round-3 tests were named as passing with their fix reverted, and they did: the freeze test froze the batch BEFORE the executor ran (so the scan never selected it), and two sweep tests asserted properties the old broken implementation also had. All three rewritten, plus a new test that runs the sweep while rows keep arriving — the exact condition the window version failed. Runbook: documented the launch gaps this review surfaced that are deliberately not auto-resolved — no operator recovery transition, the pre-existing Commission↔Allocation lock-order cycle, and why a batch frozen while `reserved` means a correlation failure rather than a routine clawback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): round-5 review — the cursor acknowledged work before doing it Round 5's headline: **Codex could not break the double-charge or double-pay guarantees** outside the accepted paymentIntents.search window — the first time a money claim has survived a round. What it did break was round 4's cursor. 1. HIGH — THE CURSOR ADVANCED BEFORE THE WORK HAPPENED. `sweepSlice` persisted the next position at hand-out time, so a crash — or any per-row Stripe/DB failure, which is caught and logged — skipped that entire slice until the next full wrap. For charges, the row can cross the 180-day horizon before the wrap and never be checked again, turning a transient failure into a permanently missed clawback. The cursor now commits AFTER the slice completes. Re-checking a row after a crash is free; skipping one loses money. 2. MEDIUM — THE CURSOR STORE COULD KILL THE WHOLE JOB. `Config.tenantId` is an FK to the seeded tenant, so a deployment that deleted that row would fail the upsert and take BOTH sweeps down globally. Cursor reads and writes now degrade to "sweep from the top" with a log instead of throwing — losing the cursor costs repeated work, not correctness. 3. LOW — the dedupe added in round 4 ran only at the final return, so the no-Stripe early return still produced one attention id per orphan ALLOCATION. Both exits now go through one finalizer. 4. LOW — dropping `failureReason` from /billing/funding was a breaking response change that left brand admins with "something is wrong, ask someone with log access". Replaced with a normalized `attentionCode` from a closed set (refunded / disputed / timed out / authorization revoked / needs review), so the UI can say something actionable without echoing Stripe object ids or internal state names. Both round-4 tests that Codex named as surviving their own reversion are rewritten: - the churn test asserted only the ORIGINAL rows and passed under the old window on most days; it now asserts every row that ever existed; - the "frozen during the run" test had NO synchronization point (and a dead variable hooking nothing). It now seeds two partners and freezes from inside the first transfer, so the second one not happening is the assertion — a real barrier at the exact production interleaving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(funding): run section H's races against real Stripe test mode Six of the twelve section-H scenarios now run against the real API — H1, H5, H6, H7, H8, H9, plus H11 observed incidentally. 21 assertions, 0 failures. H2, H3, H4, H10 and H12 have NOT been run; the runbook says so rather than implying the section is done. Adds scripts/staging-funding-races.ts (refuses a live key or a non-local DATABASE_URL) and documents the one-off ACH fixture: customer + verified us_bank_account PM + mandate via SetupIntent microdeposits. The finding worth keeping is about Stripe, not us: paymentIntents.search is eventually consistent, and a PI is not findable the instant it is created. The H1/H5 adoption path depends on that search, so a retry firing inside the indexing window will not find the intent. That makes the frozen idempotency key the load-bearing defence inside 24h, with search as the fallback past it — which is what the design already assumed, now confirmed rather than hoped. H1 itself is sound: the intent was really created, the response really lost, and the retry adopted the existing PI with create never called again (asserted by counting calls, not by reading the branch). H5 and H11 turn out to be one race resolved two ways and both landings were observed across runs. API suite still green (37 files / 285 tests), typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): round 6 — empty search is not absence, reversals stay retryable Three of the five verified defects; the two sweep ones follow separately. #75-5. releaseBatch treated an EMPTY paymentIntents.search as proof no PI exists and freed the allocations. Search is Stripe's Search API and is explicitly eventually consistent, so a PI created moments earlier is simply not indexed yet — the release could free commissions while a live PI could still debit the brand, and those commissions then landed in a new batch and were charged twice. A thrown lookup already meant "don't know"; an empty one now means the same and returns pi_not_terminal. That alone would strand orphan batches, so it is paired with a local fact that needs no search: the collector's only path to paymentIntents.create is inside the `invoicing` branch, entered by winning casBatch(reserved → invoicing). A batch we claim straight out of `reserved` therefore provably never reached Stripe and releases immediately — no search, no exposure. Anything past `reserved` holds until a PI is found or a human intervenes. #75-3. A transfer.reversed arriving before finalization linked the Payout returned 'transfer_intent_unknown', which the wrapper stamped as terminal and answered 2xx — so Stripe stopped redelivering and the only notice the money came back was discarded. It now throws FundingEventNotReadyError, reusing the existing release-claim-and-rethrow path, so the redelivery finds a linked payout. #75-4. Concurrent reversal handlers summed the ledger and wrote the payout status with no lock, so a late writer with a stale sum could regress 'reversed' to 'partially_reversed' — and 'reversed' is what gates the clawback adjustments. Now SELECT ... FOR UPDATE before the sum. On test quality: my first version of the lock test passed with the lock removed, because the handler still blocks eventually — at its UPDATE. Blocking was never the property; summing AFTER acquiring is. The test now has the blocker complete the reversal while holding the lock, so removing the lock reproduces the exact production symptom (partially_reversed instead of reversed). All three fixes verified to fail their tests when reverted. 288 tests green, typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): sweep must not acknowledge failures or order by creation The last two round-6 defects, both in the reconcile sweep. #75-1. Per-item Stripe errors are caught and logged inside the loop, then the cursor committed unconditionally once the loop finished. A failed row was therefore skipped AND passed over, and on the charge side could age out of the 180-day horizon before the cursor wrapped back — losing a clawback silently. Not fixed by withholding the cursor on failure: one permanently unreadable Stripe object would pin the cursor and starve everything behind it. Failures go into a durable retry set in the existing Config sweep state instead, come back first on the next run, and leave only on success. The cursor is free to keep moving. The set is capped and overflow is alerted rather than silent. #75-2. The cursor ordered by id, but ids are assigned at CREATION while rows join the sweep when they become eligible — a batch when it funds, an intent when it confirms. A row becoming eligible behind the cursor was only reachable on a wrap, and with a full slice of newer rows arriving every run the cursor never wrapped. Now ordered by eligibility time: (fundedAt, id) for batches, (postedAt, id) for transfers. Both are immutable once set, so a newly-eligible row is always ahead of the cursor. On test quality — this took three attempts to get honest. The first version seeded three rows and passed under BOTH orderings, because with nothing arriving the cursor wraps and covers everything anyway. The second gave the late funder a fundedAt in the middle of the seeds, so it was never actually behind the cursor. Only the third — six rows ahead of it, funding strictly later, and fresh eligible rows arriving before every run so no wrap can happen — reproduces the starvation. Both sweep tests verified to fail when their fix is reverted. 290 tests green, typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(funding): make section H prove what it claims, and say what it doesn't Round-6 review of my own script found three ways it could pass while the property was false. - H8/H9 called the inbox helpers directly and INFERRED "→409" from our own return value. A route regression acknowledging a held event with 2xx would have passed while Stripe silently stopped redelivering. It now posts a signed event to /webhooks/stripe and asserts the status code. - H5 treated `requires_payment_method` as terminal. It is not — that intent can still be confirmed and take the money — so an implementation that freed the allocations while the PI was live could have passed. - H1 asserted the retry never called `create`. That was true of one run, not of the property: the scenario backdates the DB row 25h while the real idempotency key is seconds old, so Stripe still replays it and a cold search index sends the retry down the re-POST path. One PI still exists, which is the property that matters; the route is now reported as a note. The genuine post-window path stays uncovered and the runbook says so. Adds H5b for the round-6 fix itself: with the index deliberately cold, the release must refuse to free — and the script then confirms the PI it could not see was genuinely still live, so freeing would have debited the brand for commissions already back in the pool. H5's flaky snapshot assertion is replaced by following the batch through to a terminal state, and a search that never indexes within 180s is reported loudly rather than quietly passing. 27 assertions, 0 failures against real Stripe test mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(funding): operator disposition for a batch stuck in release_requested The counterpart to "an empty search is not proof of absence". That change closed a double-charge, but it also means a batch whose PaymentIntent genuinely never existed has no automatic way out — it sits release_requested with allocations reserved until the daily stuck-release alert. A hold with no release is a leak, so this is the way out. forceReleaseBatch is the operator asserting what the system deliberately refuses to infer: that no intent exists or ever will. The runbook gives the `stripe payment_intents search` command to confirm with first, and says to let the index settle before believing an empty result. It REFUSES when the batch has a stamped PI. That case is not stuck — it is the ordinary release path — and forcing past a live intent is precisely the double-charge the protocol exists to prevent; the tests pin both the refusal and that the allocations stay reserved when it refuses. 293 tests green, typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): round 7 — five defects, all in round 6's own fixes 1. The `reserved` fast path decided from the CALLER's snapshot while the CAS accepted four source states. A row that moved reserved → invoicing between the caller's SELECT and the CAS still won, and the fast path then skipped the search and freed the allocations while a PaymentIntent was being created for it. The `reserved` claim is now a separate CAS attempted first: winning it is what proves the source state. 2. forceReleaseBatch freed the allocations and only THEN attempted its closing CAS, so losing that CAS to a concurrent release that had just found an orphan PI left a batch heading for `funded` with released, re-batchable allocations. CAS first; winning it earns the right to free. 3. The retry set could starve the entire sweep. Retries took the whole budget, so `limit` persistently-failing rows meant zero cursor capacity, a cursor reset to null every run, and nothing else ever swept — the poison-item failure round 6 introduced the retry set to prevent, back in a new costume. Retries now get at most half the budget, survivors rotate to the back so a failing head cannot hide the tail, and an overflow names the ids it is dropping instead of saying "the oldest". 4. `postedAt` is not transfer eligibility time — it is stamped before the Stripe call, so an intent whose response was lost confirms a day later and lands BEHIND the cursor. Ordering now uses a key that moves forward at confirmation, so a row can only ever be re-visited, never skipped. 5. FundingEventNotReadyError deleted the inbox claim on every attempt, so an intent whose payoutId is never populated (disputed batch, executor stops scanning it) redelivered until Stripe gave up and left NO row for the stuck-claim alert. The claim is now retained: the lease still expires for takeover, but the row persists and the 1h alert surfaces it. Two of my round-7 tests were weak and the revert checks caught them: the forceReleaseBatch test hit the early status guard and never reached the ordering code (fixed with a seam that stages the race between the read and the CAS), and the eligibility test let the cursor wrap and pick the row up by accident (fixed with churn and explicit updatedAt). All five fixes now fail their tests when reverted. 298 tests green, typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): round 8 — five defects, all in round 7's own fixes 1. forceReleaseBatch CASed on status alone, but a concurrent releaseBatch can find an orphan PI and STAMP it while the row stays `release_requested`. The status-only CAS still won, freed the allocations, and left a batch heading for `funded` whose commissions were already back in the pool. casBatch gained an `expect` predicate and the CAS now requires stripePaymentIntentId to still be null at the moment it wins, not merely when it read. 2. The retry set's "rotation" rotated nothing: `commit` reordered the stored set, but selection re-filtered `rows` and re-picked the same head every run, so the tail was never retried. Selection now reads the stored order. 3. At limit 1 the retry set took the only slot, `remaining` was 0, and one permanently-failing row starved every healthy cursor row forever — the poison-item failure for the third time. Retries never take the last slot now; at limit 1 they get none and are reached by the cursor instead. 4. The `updatedAt` ordering rested on "it only ever moves forward", which was false: every writer used the app's `new Date()`, so a node running a minute behind could finalize a row BEHIND the cursor and strand it. All HostedFundingTransfer.updatedAt writes now use db.fn.now() — one clock, so the premise actually holds. 5. The stuck-claim alert measures age from `processedAt`, which a claim takeover REFRESHES, so an event redelivered every few minutes postpones its own alert indefinitely. Rather than depend on that timing, reconcile now detects the underlying condition directly: an intent posted with no linked Payout for over an hour cannot accept a reversal and is reported on its own. On test honesty: the rotation test is labelled coverage, not a mutation killer. I could not isolate rotation — with a cursor slot free the cursor reaches the second row on a wrap regardless of selection order, and two attempts at forcing it both passed with the fix reverted. It proves the weaker property that neither row is abandoned; the rotation rests on inspection, and the test says so instead of implying otherwise. 302 tests green; staging matrix 28 assertions, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(funding): round 9 — per-object sweep scheduling replaces the cursor Three round-9 findings shared one root: a global high-water mark makes an ordering claim that concurrent writers keep falsifying. CURRENT_TIMESTAMP is transaction-START time, so commit order could still land a row behind the persisted cursor; the limit-1 retry budget was 0, so under churn a poison row was never re-attempted; and retry-set overflow trimmed the untouched head of the list, not the oldest-attempted. Scheduling state now lives on the swept rows themselves (sweepDueAt / sweepLeaseAt / sweepLeaseToken / sweepFailCount). Claims are `for update skip locked` under a per-run lease token, Stripe reads happen outside the claim statement, and every claimed row is acknowledged under that token. Success and failure reschedule IDENTICALLY, so selection is pure least-recently-visited rotation: a poison row costs one slot per rotation and escalates through sweepFailCount instead of through priority, and there is no high-water mark for a clock-skewed timestamp to strand a row behind — a rank, unlike a mark, is recomputed from live rows every run. The cursor, the retry set, their Config plumbing and their budget arithmetic are deleted. Every prior coverage test passes unchanged; the new lease and uniform-reschedule tests were both checked by reverting the mechanism under them and watching them fail. Migration adds the four columns to both hosted-only sidecar tables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(funding): round 9 — forceReleaseBatch verifies its own premise The round-8 null-id fence closed "stamp commits before the force CAS", not "releaseBatch discovered a PI it has not stamped yet": the force still won its CAS with the id null, freed the allocations, and the discovered PaymentIntent could then succeed against commissions that were already re-batchable — the double-charge, with only an alert left. Two additions close the discovery window: - A QUIET GATE: the force refuses (too_recent) until the batch has sat unchanged in release_requested for an hour. A PaymentIntent can only be created while a batch is in the collector's invoicing branch, so an hour of quiet means any existing intent predates the stuck state by an hour — far past Stripe's search-indexing lag. - ITS OWN SEARCH, with the Stripe client now REQUIRED: given the gate, the search is conclusive. Found → refuse and stamp the id so the ordinary release path terminalizes it; search failed → cannot_verify, nothing freed. This is also the round-9 §0.2 item — the function no longer merely checks that a DB column is null while being described as verified. The search guard was checked by reverting it and watching its test fail. Funding suites: 94 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(funding): round 10 — one clock for leases, two eyes on every debit Codex's round-10 pass on the funding rail, all verified before fixing: - A stalled collector pass could create a PaymentIntent from a STALE invoicing snapshot after a release claimed the batch — recoverable only through the orphan-cancel compensation, whose cancel can lose to a bank debit already processing. The collector re-reads the LIVE status immediately before creating; the stale window shrinks from "whole pass" to "one Stripe call", which the force-release quiet gate then covers entirely. - A funding PI's only stamp is mutable metadata — cleared, it was invisible to forceReleaseBatch's search (the exact failure the transfer-side fixes closed with transfer_group, which PIs lack). The force now also LISTS the tenant customer's PaymentIntents — metadata-independent — and refuses on any non-canceled intent matching the batch's exact amount+currency since the batch was created. One-open-batch-per-tenant+currency is what makes that match decisive. - Sweep leases now use the DATABASE clock on both write and comparison: a clock-fast worker could strand a row behind a future lease, a clock-slow one could be reclaimed mid-flight. And a never-swept row's eligibility hint is clamped with least(..., now()) so a future-skewed timestamp means "due now", not "behind every rescheduled row forever". - The force-search test fixture now carries the metadata stamp the production query actually matches on — it previously fed the mock a PI the real search could never return. Both new guards were checked by reverting them and watching their tests fail. Full API suite: 37 files green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-pay (#10) (#73) * fix(payouts): durable transfer intent — stop the direct-Connect double-pay (#10) runPayouts called stripe.transfers.create INSIDE the caller's tenant transaction, keyed on a payout id minted in that same transaction. Two double-pay paths followed: 1. the transfer succeeds and the COMMIT then fails — the Payout row and the paid commissions roll back, the money is gone, and the next run regroups under a NEW payout id, hence a NEW idempotency key, hence a second transfer; 2. an ambiguous Stripe error (timeout — the transfer may or may not exist) marked the payout failed, left the commissions approved, and the next run retried under a new key. This is a live path: self-host Connect payouts and the deliberate OPENPARTNER_ALLOW_UNFUNDED_CONNECT_PAYOUTS=1 override. It is not behind the funding flag. A deterministic key over the commission set does NOT fix it — if any commission is approved between attempts the set changes, the key changes, and the second transfer pays the overlap again. So mirror funding/executor.ts, which already had the right shape: - payouts.ts is now a PLANNER. It writes the Payout row as an intent (metadata.transferState='intent', frozen amountMinor + destination) and freezes its commission set by claiming the rows — Commission.payoutId stamped while status stays 'approved'. Every planner lookup filters payoutId is null, so claimed commissions can never be regrouped into a second, larger transfer. No Stripe call happens inside the transaction. - payout-transfers.ts is the EXECUTOR. Outside any transaction, on the privileged pool: preflight (set unchanged? partner still ready?), CAS intent→posted, POST with the durable key payout_<payoutId>, finalize in a short transaction, webhooks strictly after the commit. - Ambiguous outcomes stay 'posted': inside Stripe's ~24h window a retry replays the frozen key; past it the intent goes reconcile_required and is resolved by paging transfers.list({transfer_group}) for our metadata stamp — never a blind re-POST. Proven absent → re-armed as 'intent'. - Definite 4xx fails the payout and releases the claims so the next run regroups. A transfer that comes back reversed is never recorded paid. Also: the amount now comes from the rows actually claimed rather than a separately-snapshotted SUM, and withTenantTransaction (tenancy.ts) is the one way to open a tenant-scoped transaction — the scheduler and the admin POST /payouts/run both use it, the latter committing its intents before any money moves instead of borrowing the request transaction. New scheduler job payout-transfers (*/15) retries and reconciles intents left open by a crash, a timeout, or an unready partner. 16 DB-backed tests with a Stripe mock cover every path above, including the commit-fails-after-transfer and set-change scenarios. Operator doc: docs/direct-connect-payouts.md (state machine, recovery SQL, and the staging checklist to run in Stripe test mode before this pays anyone). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): adversarial-review fixes — 409 double-pay, stale replay, missing interlock Codex review of #73 refuted both claims the PR made. Three real defects, all of them ways the "cannot double-pay" property failed. 1. A 409 IDEMPOTENCY CONFLICT WAS TREATED AS PROOF OF ABSENCE. Every 4xx classified as definite → the intent was failed and its commissions released. But 409 `idempotency_key_in_use` means another request is using this key RIGHT NOW and may well succeed; releasing lets the planner regroup those commissions under a NEW key while the first transfer lands. That is exactly the double-pay this PR exists to stop. 429 had the same problem (no transfer, but the intent was destroyed). Both are now ambiguous: the intent stays posted and the retry replays the frozen key or reconciles by listing. The funding executor never released on these; the divergence was mine. 2. TWO WORKERS COULD RE-POST ONE INTENT CONCURRENTLY. A `posted` intent past the 60s cooldown fell through to transfers.create with no claim, and the admin route, the weekly payouts job and the 15-minute executor job hold DIFFERENT locks, so they can overlap. The retry now takes a lease by swapping the exact `postedAt` it read — `posted → posted` matches for everyone, but only one worker can swap a given timestamp. 3. A RETRIED KEY REPLAYS A STALE OBJECT. Stripe answers a repeated idempotency key with the response it stored at creation, so `reversed` in that body is false even if the transfer has since been clawed back — and finalizing on it overwrote a reversal webhook's `failed` with `paid`, marking commissions paid on money that came back. Any attempt past the first now re-reads the transfer from Stripe before believing it, and a re-read that fails records nothing. Also: the frozen commission set had no protection from the OTHER side. `interlockCommissionReversal` only knew about funding allocations, so the admin reverse endpoint and the refund clawback would flip a commission claimed by a posted intent — Stripe still gets the frozen amount while fewer commissions are marked paid. It now holds commissions claimed by an open Payout intent too, which gives every reversal path the guard for free. (Preflight only helps before the first POST.) The PR also claimed money can never be stranded, which was overstated: a reversed transfer deliberately leaves its commissions claimed with no automatic path back. That case now has a documented operator disposition rather than an implied one. 7 new tests: 409 holds the claim, 429 holds the claim, two workers retry → one POST, stale replay is re-read and never resurrects a reversal, interlock holds a claimed commission, and a canceled intent frees it again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): split the lease clock from the idempotency-window clock Self-caught regression in the previous commit. The retry lease swapped `postedAt` — which is ALSO what Stripe's ~24h key-retention window is measured from. A steadily-retried intent therefore refreshed its own window on every attempt, never reached `reconcile_required`, and once Stripe pruned the key a re-POST would have created a SECOND transfer. The fix for one double-pay reintroduced another. Two clocks now, with distinct jobs: postedAt — when the frozen key was FIRST used. Never moves. Anchors the idempotency window. leaseAt — when the last attempt claimed the intent. Moves on every retry; it's the compare-and-swap token that keeps two workers off one key, and the cooldown reads it. Re-arming after a proven-absent reconcile clears both. Two regression tests: an intent posted 25h ago but retried 2 minutes ago still reconciles by listing instead of re-POSTing, and the cooldown reads the lease clock rather than the first post. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): round-2 review — lease vs expiry race, interlock check/use gap, error type Second adversarial pass over the fixes themselves. Six findings, all verified against the code. 1. THE EXPIRY PATH COULD STEAL A WARM LEASE. The window check ran before the cooldown and CASed on transferState alone, so a worker that had just leased and was inside transfers.create could have its intent moved to reconcile_required, listed and finalized by another worker — and then its own POST landed on a key Stripe had pruned, creating a SECOND transfer. Cooldown now runs FIRST: a warm intent is left alone whatever its age. 2. CHECKING THE AGE THEN POSTING IS A TOCTOU AGAINST STRIPE'S RETENTION. A worker that measured "23h59m, fine" can have its request arrive after the key was pruned. The key is now treated as spent an hour early (KEY_SAFETY_MARGIN_MS), so every POST we authorize lands with room to spare instead of racing the expiry we just measured. 3. THE REVERSAL INTERLOCK HAD A CHECK/USE GAP. interlockCommissionReversal is a read; the status flip is a separate write. The planner can commit an intent and claim the commission in between — after which the transfer goes out for the frozen amount and the commission is reversed, never paid. Both reversal paths (admin route, refund clawback) now re-assert the guard INSIDE the UPDATE, so whichever statement lands second sees the other's work. 4. THE IDEMPOTENCY-ERROR EXCLUSION CHECKED THE WRONG PROPERTY. stripe-node puts the wrapper class on `type` ('StripeIdempotencyError') and the API's string on `rawType` ('idempotency_error'). Matching `type` against the API string never fired, so a 400-level idempotency error (a key reused with different parameters) was still classified definite and released the claims. 5. 500 STUCK INTENTS COULD STARVE EVERY OTHER TENANT. The scan is global, capped at 500 and ordered by createdAt, so a wedged backlog monopolized every pass. Ordered by least-recently-attempted now: each attempt bumps leaseAt, so a stuck intent falls to the back on its own. 6. Smaller: a transfer we can see but cannot re-read now persists its error and escalates to an ALERT after 5 attempts instead of retrying silently forever; an interlock whose Payout row can't be resolved fails CLOSED (left join, not inner) since Commission.payoutId has no FK; and a hand-repaired `leaseAt: null` takes the IS NULL branch instead of `= NULL`, which never matches and would have wedged the intent. Not applicable from the review: it flagged an upgrade hazard for rows written by the intermediate commit a38732e. That commit only ever existed on this unmerged branch — no deployment has ever written a `postedAt` under those semantics, and `main` has no transferState at all. 6 new tests, including the warm-lease and check/use races. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): round-3 review — ABA on the expiry CAS, and the key was never fresh Third adversarial pass. Two of these are the deepest defects found in this branch so far, and both were introduced by earlier fixes. 1. CRITICAL — ABA ON THE EXPIRY TRANSITION. The posted → reconcile_required CAS checked only `transferState`. A worker holding a stale scan snapshot could arrive after the intent had been reconciled, re-armed and posted afresh by someone else, see `posted` again, and steal that NEW generation out from under a live transfers.create. The victim's finalize then lost its CAS, so a real transfer went unrecorded — and a later POST could pay it again. The transition is now scoped to the exact `postedAt` the worker observed, so a stale reader loses. 2. CRITICAL — "A GENUINE FIRST ATTEMPT" WAS NEVER TRUE. After listing proved no transfer existed, the re-arm cleared our own clocks and re-posted `payout_<payoutId>` — the SAME key. Stripe's retention is anchored to when that key was FIRST used, not to our local timestamps, so a re-armed POST inside the retention replays the stored outcome (including a stored failure) rather than doing anything, and the local window restarts — with no ceiling. Proving absence is exactly the evidence needed to make a NEW key safe, so the re-arm now bumps a `keyGeneration` and posts under `payout_<id>_g<N>`. Generation 0 keeps the original key so anything already posted under it still replays. 3. HIGH — THE REFUND PATH SILENTLY DROPPED LATE-HELD COMMISSIONS. The guarded UPDATE added in round 2 can refuse a commission claimed after the interlock read, but the return still reported only the interlock's own held count. Those commissions were reported as neither reversed nor held: the refund looked fully handled while a commission for refunded revenue stayed payable. They are now counted and alerted. 4. MEDIUM — THE SCAN ORDER MIXED TWO TIMESTAMP FORMATS. `"createdAt"::text` renders `2026-08-09 20:04:15.501+00` while the metadata clocks are ISO `...T...Z`, and a space sorts before `T` — so every never-attempted row jumped ahead of every attempted row sharing a date, regardless of time. With a 500-row cap that is queue starvation. All three keys are now rendered in the same ISO shape, pinned to UTC. 5. MEDIUM — an unknown `transferState` (rolling deploy, hand-edited row) was treated as "not open" and released the reversal hold. The guard now keys on the TERMINAL states, so anything unrecognised fails closed. 6. LOW — a failed transfer re-read could write its stale error onto a row another worker had already finalized; that write is now predicated on still holding the intent. Not duplicated here: the review also flagged the funding executor's un-paginated 100-transfer reconcile listing. That is real, and it is exactly what PR #71 fixes — both need to merge. 5 new tests, including the ABA staged directly against the CAS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): round-4 review — make keyGeneration a real epoch, not just a key suffix Round 3 introduced generations to stop a re-armed intent reusing a key Stripe still retained. It fixed that, and opened a new dimension that nothing was fenced on. Three critical consequences, all verified: 1. THE RE-ARM WASN'T FENCED. `reconcileIntent` computed `next = read + 1` from its own snapshot and CASed on state alone. A reconciler resuming from a stale read could therefore hand generation N+1 back out AFTER another worker had already posted under it — so a live generation became reusable, and once Stripe pruned that key the next POST created a second transfer. The re-arm now CASes with the generation it read. 2. GENERATIONS WERE INDISTINGUISHABLE AT STRIPE, AND DUPLICATES WERE SILENTLY COLLAPSED. transfer_group stayed the payout id (correct — one listing should see every generation) but the metadata carried no generation and reconcile took the FIRST match. Stripe lists newest-first, so a late transfer from generation N arriving after N+1 succeeded would have been hidden behind the newer one: two real payments, one recorded, nothing detecting it. Transfers now carry `openpartner_key_generation`, and reconcile collects ALL matches across all pages: zero → fenced re-arm, one → finalize, MORE THAN ONE → write no ledger, alert, and leave it for a human. 3. FINALIZATION WASN'T FENCED EITHER. A worker holding a transfer from generation N could finalize it against an intent that had since moved to N+1 — recording the wrong transfer while N+1's own transfer stayed live. Every post-Stripe mutation (finalize, reversed-transfer close, definite failure, ambiguous error, retrieve failure) is now scoped to the generation it belongs to. Also: - A malformed `postedAt` produced NaN, and every comparison with NaN is false — so a corrupt timestamp sailed past BOTH safety checks into the re-POST path, the one place we must never go on a guess. Unknown age now reconciles by listing, which is always safe. - The refund path attributed every missed row to "a payout intent claimed it", so a commission concurrently paid or reversed by another actor produced a false alert AND was double-counted against alreadyPaid. The missed rows are now re-read and classified. - The reversal UPDATE guard covered the direct rail but not funding: a reservation could insert an allocation after the interlock read, and the funded executor would then overwrite `reversed` with `paid`. It now excludes live allocations too. 4 new tests, including duplicate detection and a superseded-epoch finalize. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): round-5 review — the lease was shorter than Stripe's own request budget Fifth adversarial pass. Three criticals, all in round-4's epoch wiring, plus one that has been latent since the lease was introduced. 1. CRITICAL — THE LEASE COULD EXPIRE WHILE THE POST WAS STILL IN FLIGHT. `POST_COOLDOWN_MS` was 60s, but our Stripe client passes no options and stripe-node defaults to an 80s timeout with 2 network retries — so a single transfers.create can occupy ~4 MINUTES. Another worker would declare the lease cold, reconcile, prove absence and re-arm, and then the original transfer landed alongside the new generation's. Two transfers, one recorded, and the row leaves the scanned states so duplicate detection never runs on it. The money call is now explicitly bounded (20s, 1 retry) and the lease (180s) is required to dominate that budget — with a test asserting the relationship rather than the constants. 2. CRITICAL — TRANSFERS WERE STAMPED WITH THEIR GENERATION AND THE STAMP WAS IGNORED. Reconcile matched on payout id only and finalized with the ROW's current generation, so a generation-0 transfer discovered while the row was on generation 1 passed the generation-1 fence and was recorded as the current epoch's — the exact invariant the stamp was added for. It now reads the stamp: a transfer from a superseded epoch means an abandoned attempt DID move money, which is the duplicate case in slow motion, and it freezes instead of finalizing. 3. CRITICAL — `meta` STAYED THE PRE-CAS SNAPSHOT. Round 4's own conclusion was "after a CAS, use the returned row"; I only reassigned `payout`. Everything downstream — the idempotency key, the Stripe stamp, every fence — kept deriving from the stale object, so a worker could POST generation N's key while the database said N+1. `meta` is refreshed from every CAS now, and the `intent → posted` transition is fenced on the observed epoch too. 4. HIGH — the fence used `("metadata"->>'keyGeneration')::int`, and a cast RAISES on a non-numeric value where `coalesce` cannot catch it. One malformed metadata blob would make every fenced CAS throw and wedge the intent permanently — possibly after money moved. Now a text comparison, with the decoded generation validated; an unreadable epoch refuses to act and alerts rather than guessing. 5. HIGH — duplicate detection was a permanent hot loop: it left the intent in `reconcile_required`, which is scanned every 15 minutes, so Stripe was re-listed and the alert repeated forever — and 500 such rows would consume the entire scan cap and starve other tenants. Duplicates now park in a `duplicate_review` state the executor does not scan, with the disposition documented. 6. HIGH — the transfer webhooks ignored the epoch entirely, applying by payout id alone: a `transfer.reversed` from a superseded generation could mark a legitimately paid payout failed. They now apply only to the transfer the payout actually recorded. 7. MEDIUM — the refund path dropped a commission that became `paid` between its two reads from BOTH totals, reporting a cleanly-handled refund when money had just moved; and it labelled every held row as payout-claimed when it could equally be allocation-held. Codex confirmed generation-0 compatibility holds throughout (absent key matches every fence), and that `missedIds` is correctly bounded. 4 new tests. The round-4 seam test that gave false confidence here — it handed finalize the old generation explicitly, which the real reconcile path never did — is replaced by one that goes through the executor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(payouts): run the direct-Connect matrix against real Stripe test mode The staging checklist in this doc was the stated gate for this PR and had never been run — neither money path had ever touched Stripe. All six scenarios now pass: 37 assertions, 0 failures. Adds scripts/staging-direct-connect.ts so the matrix is a command rather than a manual checklist. It refuses a live key or a non-local DATABASE_URL, since it truncates tables and moves money. Three things only a real run could establish: - Replaying the frozen idempotency key after a lost response returns the SAME transfer. That is the assumption the whole intent design rests on, and it was previously only asserted against our own mock. - Past the 24h window the executor calls transfers.list and never transfers.create — verified by counting calls on a wrapped client, not by reading the branch. - An unready destination produces a real Stripe 400, classified definite, releasing the claims. The mock's 400 was our own invention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): round 6 — ambiguity freezes instead of re-arming Three verified defects, two of them able to pay a partner twice. 1. The safety argument rested on POST_COOLDOWN_MS exceeding the Stripe request budget. That was false: stripe-node implements `timeout` as req.setTimeout, a socket-INACTIVITY timeout that resets after each request stage — its own source says so, contrasting Node with fetch. A slow-but-progressing POST has no wall-clock bound and can outlive any cooldown, get declared cold, be reconciled, re-armed at a new generation, and land as a second transfer. No constant fixes this, and neither does a local deadline: aborting the await cannot retract a request Stripe already received. So the executor no longer re-arms at all. An empty transfers.list means UNKNOWN, never ABSENT — the intent is HELD on its generation with commissions frozen, later ticks keep listing (a straggler still finalizes normally), and only an operator may authorise a fresh key. POST_COOLDOWN_MS survives as scheduling and is documented as carrying no correctness weight. Adds releaseIntentForRetry and disposeIntent, because a hold with no release is a leak. Both are generation-fenced. 2. finalizeTransfer re-read the live transfer only when attempts > 1, so a first-attempt transfer reversed between the create response and the DB write was recorded paid from a stale body — and the reversal webhook could not repair it, because it matched on a stripeTransferId that is not stamped until finalization. Now it always re-reads, and the webhook matches an unstamped payout on the transfer's immutable metadata (openpartner_payout_id + openpartner_key_generation), terminalizing the intent so the stale finalizer loses its CAS. 3. markDuplicateReview CASed on state alone, so a reconciler resuming from a stale snapshot could quarantine a live newer generation — failing a payout whose single transfer had legitimately moved money. Fenced on the generation the caller observed. The Stripe mock had no `retrieve` at all, which is what let defect 2 hide: it modelled an API whose live object cannot be read. Added, with a seam for "reversed after the create response". Both new tests verified to FAIL when their fix is reverted. 298 tests green, typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(payouts): make the matrix prove concurrency and the new hold Two honest problems with the script as written, both found by the round-6 review of my own work: - Scenario 6 reported SUCCESS when the planner returned no payout at all, so a misconfiguration that never reached Stripe was indistinguishable from Stripe definitively rejecting the transfer. That is now a failure. - The header claimed the script covered round 5's lease-vs-request-budget property. It never did — the injected failures happen after the real request has fully returned — and that property turned out to be unprovable anyway. The header now says what the script does NOT prove. Adds the two scenarios the rework needs, run against real Stripe: - 7: two genuinely concurrent executors over one intent → exactly one transfer, one recorder, one paid commission. First concurrency coverage this matrix has had; every earlier run was single-threaded, which is precisely why the CAS and lease went unexercised. - 8: past the window with nothing at Stripe → three ticks must not post, the intent stays reconcile_required on generation 0 with commissions frozen, and only releaseIntentForRetry lets it post, exactly once. 50 assertions, 0 failures. Doc updated: the state machine no longer claims listing proves absence, and the manual-SQL release is replaced by the two fenced operator calls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(payouts): delete the cooldown-arithmetic test — it guarded nothing Both round-6 reviews independently called for this one's removal, and I had cited it to the user as the guard on POST_COOLDOWN_MS. It parsed three constants out of the source and asserted `cooldown > timeout * (retries + 1)`. That is worthless twice over: - It asserted a guarantee that does not exist. stripe-node implements `timeout` as req.setTimeout — a socket-inactivity timeout that resets after each request stage — so no arithmetic over those constants bounds the wall-clock life of a POST. - It was not a mutation killer even for the fix it claimed to protect. Restoring the old 60s cooldown still passes `60 > 40`, and nothing checked the request options ever reached Stripe. The invariant that does hold is now covered instead: the executor never re-arms on elapsed time, so a cold lease can never authorise a new key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): round 7 — four defects, two of them in round 6's own fixes 1. The metadata fallback I added in round 6 had the race it was closing. The exact-id match and the fallback are two statements, and the executor commits finalization in its own transaction — so it could stamp stripeTransferId BETWEEN them: the first missed (id null), the fallback missed (it requires id null), and the event fell through to "unmatched" and was acknowledged 2xx. Reversed transfer, payout paid, no second chance. The transfer case now takes the payout row lock first, so we see either the pre- or the post-finalization state and never the gap. 2. disposeIntent accepted duplicate_review and released the claims — a straight double-pay with no concurrency needed. A duplicate is disposed of by reversing the SURPLUS and keeping one transfer, so those commissions have been paid; releasing them let the planner pay them again. That state now has its own function, resolveDuplicateReview, which takes the operator's actual disposition: a kept transfer records the ledger against it and marks the commissions PAID, all-reversed returns them to the pool. 3. releaseIntentForRetry could re-arm while a reconciler held a transfer it had just found. The generation fence does not cover that — the reconciler has written nothing yet, so it is still on the observed generation. It now lists the transfer group itself and refuses on positive evidence. That is not proof of absence (nothing is), but the transfer a reconciler found is by definition visible to a listing. 4. A reversed transfer left confirmed+failed with commissions frozen "for operator disposition" and no function accepting that state. disposeIntent now takes it, guarded by a new notStatus predicate so a payout whose money reached the partner can never be released back into the pool. Every guard verified to fail its test when reverted. 303 tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(payouts): pick partner2's payout explicitly — [0] was order-dependent CI caught what my local run did not. The dispose earlier in the test returns the first partner's commission to the payable pool, so the following plan() produces a payout for BOTH partners and their order is not guaranteed. Indexing [0] happened to pick the right one locally and the wrong one in CI. Selects by partnerId instead. Ran three times locally to confirm it is stable rather than lucky. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(payouts): exercise the round-7 listing guard against real Stripe releaseIntentForRetry now refuses to re-arm when a transfer is visible in the group. The staging matrix called it without a Stripe client, so that guard was skipped in the only place it runs for real. Passes the client. 50 assertions, 0 failures against test mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): round 8 — operator actions must verify their own premise Round 6 removed "an empty search proves absence" from the automatic paths. Round 7 then built operator paths that act on an unverified human assertion, which is strictly weaker than what had just been removed — and a human typo is silent where a Stripe read is not. This applies that rule. 1. disposeIntent released on `status !== 'paid'`, but finalizeTransfer records confirmed+failed for ANY non-zero amount_reversed — including a PARTIAL clawback. A $10 reversal on a $50 transfer leaves $40 with the partner, and releasing those commissions let the planner pay the full $50 again: $90 out for $50 owed. It now retrieves the stamped transfer and refuses ('money_with_partner') unless it is fully reversed. 2. resolveDuplicateReview validated nothing. `{allReversed}` while transfers were live released the commissions and double-paid; a typo'd `{keptTransferId}` recorded the payout paid against a transfer that does not exist AND wedged the row where no operator function accepted it. Both dispositions are now checked against the transfer group: a kept transfer must be present and unreversed, and all-reversed is refused while any transfer still holds money. My own test had encoded the typo case as success — it now asserts the refusal. 3. The listing guard on releaseIntentForRetry only ran `if (stripe)`, and the documented operator call in the runbook omitted it — so the guard was absent in the one usage operators would copy. The parameter is required now. An optional security parameter is one someone forgets. 4. That guard also treated exhausting its 20-page budget as absence. Running out of pages is not the same as finding nothing; it returns 'cannot_verify' instead, as does any failed read. The staging matrix only ever tested that a re-arm was ACCEPTED, so deleting the guard produced the same result. It now also proves the refusal against real Stripe once a transfer exists: 51 assertions, 0 failures. Runbook updated — every operator call now shows the required client and what each refusal means. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payouts): round 9 — the reversal webhook can now always find the payout The guarantee the finalizer relies on — "the reversal webhook can always find this payout" — had two holes, and both are closed by making the IMMUTABLE identifiers load-bearing instead of the mutable ones: - duplicate_review was invisible to the webhook: its fallback accepted only posted/reconcile_required, so a reversal landing while an operator resolved a duplicate was dropped as unmatched — and the stale resolution then recorded the reversed transfer as the kept one, paid. The webhook now RECORDS reversal activity on a parked review (without claiming or terminalizing — siblings may still hold money) and moves a duplicateReviewNonce; resolveDuplicateReview fences both commits on the nonce it observed, loses to newer activity, and answers review_moved. Partial reversals count, and the generation fence deliberately does not apply — a duplicate group spans generations by construction. - metadata.openpartner_payout_id is mutable at Stripe, and every guard filtered on it: a cleared-metadata transfer vanished from releaseIntentForRetry (re-arm posts a second transfer), from {allReversed} (vacuously satisfied by an empty group), and from the webhook's identification itself. Membership is now the transfer_group — set at creation, immutable, stamped with the payout ULID — everywhere. Also from round 9's findings, in the functions option B keeps as its apply-step validation: - disposeIntent with NO stamped transfer (the common ambiguous case) released claims with no Stripe read at all. It now lists the whole group and refuses while any member holds money; empty is the operator's documented risk decision, not proof. - A kept transfer must match the intent's FROZEN amount, currency and destination — present-and-unreversed alone let a one-cent transfer mark a $50 payout fully paid — and every sibling must be fully reversed. - stripe is REQUIRED in both signatures; the runbook said it was while the types said otherwise, and its duplicate_review section told operators to release the claims — the exact double-pay resolveDuplicateReview exists to prevent. Both corrected. The nonce fence and the lease guard were checked by reverting them and watching their tests fail. Full API suite: 316 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(payouts): round 9 — a ghost transfer stamp no longer wedges disposal A confirmed/failed payout whose stamped stripeTransferId answers resource_missing — a hand repair, or a payout wedged by the pre-round-8 typo path — returned cannot_verify forever: fail-closed with no exit but raw SQL. A stamp that cannot be dereferenced proves nothing either way, so disposeIntent now falls back to the same whole-group verification as the unstamped case: a live member still refuses (the ghost stamp does not excuse ignoring real money), an empty group opens the operator's documented risk decision. Any other retrieve error still refuses. The group pagination is now one shared helper (listTransferGroup) used by disposeIntent and resolveDuplicateReview, rather than two copies that could drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(payouts): round 10 — authenticate what the group asserts, fence what it cannot Codex's round-10 pass refuted six of the seven claims put to it. The fixable set, all verified before fixing: - The review nonce was the Stripe EVENT id, and Connect events return before the event-id dedupe — so a redelivered old event wrote its id back and a stale resolution fenced on it committed: an ABA. The nonce is a fresh ulid per write now; no observed value can ever return. - Group membership is unauthenticated: anything on this Stripe account can set transfer_group to our payout ULID, and dropping the metadata filter (round 9) let reconcileIntent FINALIZE such a member. Reconcile now authenticates against the frozen intent — amount, currency, destination — plus an immutable time bound: transfer.created cannot predate the current generation's postedAt, which also defeats a FORGED generation stamp. Failures park for a human, never finalize. - Identity resolution preferred the mutable metadata stamp over the immutable group, so a forged openpartner_payout_id could redirect a reversal into unresolved-tenant 2xx. transfer_group wins now. - A partial reversal (transfer.updated, reversed:false, amount_reversed>0) re-asserted `paid` — flipping even a failed payout back. Status is never overwritten on a partial; it alerts instead. - disposeIntent skipped group verification when the stamped transfer was fully reversed — a late duplicate holding the full payment was invisible. The group is verified ALWAYS. - listTransferGroup treated an empty page with has_more as a complete group; it now refuses (cannot_verify). - frozenAmountMinor: Number() coercion accepted amountMinor:true as 1 cent; only an actual finite number counts now. - NEW transfer.created handler (detector only, no state writes): the alarm for the honest prove-absence limit — a POST landing after a disposition raises transfer_created_orphan instead of passing silently. Register transfer.created on the webhook destination. Deliberately NOT closed here (documented in the handoff): an unbounded in-flight POST landing between a verified action's listing and its CAS — that is the prove-absence limit itself; the detector above is its alarm and option B's request ledger is its tombstone. The ABA test replays the SAME event id after a newer event; the authenticity tests stage a one-cent foreign member and a backdated forged-generation member. Full API suite: 325 green twice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ump, real PKs (#8) (#74) * fix(export): complete the portability promise — missing tables, SQL dump, real PKs (#8) CLAUDE.md and the README promise every table exports to CSV + JSON + SQL with a re-importable round-trip. Three ways it wasn't true: 1. The exported set omitted PartnerProgram, PartnerCommission and Coupon. A restored instance had the partner roster but no program grants, no record of what any partnership was agreed at, and no coupon → (partner, program) mapping — so coupon-driven conversions could not be re-attributed. Added, in FK-safe positions. 2. No SQL dump existed at all, though the contract names it. 3. Import hardcoded `id` as the conflict target, but PartnerCommission is keyed on partnerId — adding it naively would have thrown on every re-import. Each table now carries its real primary key in EXPORT_TABLES, and the list doubles as the import order (parents first). The round-trip test found two more, both live bugs: - A JS array coming out of `select *` is ambiguous: Program.commissionRule is a jsonb array, Program.categories is a text[]. The driver renders a bare array as a Postgres array literal, so importing ANY program with compound commission rules failed with "invalid input syntax for type json". Both paths now read the live column types and render each correctly. - Same ambiguity in the SQL dump: a text[] column got `::jsonb`. New: GET /export.sql (full bundle) and GET /export/<Table>.sql. The dump is portable by default — rows are written under a psql variable, so one file restores anywhere: psql "$DATABASE_URL" -v tenant_id=default -f openpartner-export.sql It opens a transaction, sets app.tenant_id so it works on the RLS-scoped app role as well as the privileged one, and every statement is ON CONFLICT DO NOTHING so a restore is idempotent and resumable. `?tenantId=<id>` bakes a literal instead, for clients that don't run psql meta-commands. schemaVersion 1 → 2. POST /import still accepts v1 bundles: an export sitting on someone's disk must not become worthless because we added tables. SUPPORTED_IMPORT_VERSIONS only ever grows. Tests (export-roundtrip.test.ts, 11): seed one row in every exportable table → export → wipe → restore → compare, once through JSON and once through the SQL dump actually executed against Postgres (including the portable form put through psql's own substitution). Plus idempotency on both paths, v1 acceptance, PK/order invariants, and escaping of quotes, jsonb arrays, text[] and nulls. Also: the portal export page listed 9 of the 14 tables and offered no SQL; docs/data-portability.md is the format contract (bundle shape, table list with keys, restore procedures, and how to add a table). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(export): adversarial-review fixes — dump injection, wrong restore id, string mode Codex review of #74 found three real defects. All three are in the part of the change that generates a file someone else later executes, which is exactly where I should have been most careful. 1. COMMAND INJECTION via `?tenantId=`. The parameter was accepted verbatim and interpolated into a `--` header comment. psql meta-commands are line-oriented, so a value containing a newline and `\! <shell command>` produced a dump that runs that command on the machine doing the restore — the documented workflow. The values were escaped; the comment was not. The id is now VALIDATED, not escaped (`[A-Za-z0-9_-]{1,64}`, refused with a 400), header comments strip newlines regardless, and buildSqlDump asserts the same rule so no future caller can reintroduce it. I wrote exactly this guard for Stripe ids on another branch and missed it here. 2. THE DOCUMENTED RESTORE COULD NEVER HAVE WORKED. The fallback was `\set tenant_id 'default'` and the docs said `-v tenant_id=default` — but `default` is the tenant SLUG. `tenantId` is a foreign key to `Tenant.id`, whose seeded value is `01J0000000DEFAULTTENANT0000`, so the first row failed the FK and rolled the whole restore back. The fallback is now DEFAULT_TENANT_ID, and the docs/README/portal say "destination tenant id", not a slug. The old test hid this by substituting the real id before executing; there is now a test that runs the dump the way `psql -f` with NO -v does, using the file's own fallback. 3. The escaping assumed `standard_conforming_strings` is on (the default since PG 9.1) but never said so; with it off, a backslash in exported data escapes the quote doubling and breaks out of the literal. The dump now pins the setting in its preamble. Also from the same review: normalizeRow revived ANY ISO-shaped string as a Date, so a partner named "2026-08-09T00:00:00" was corrupted on import and a lookalike that isn't a valid date could fail it. Now that the importer reads column types, the coercion is gated on the column actually being a timestamp. Tests: no -v restore path, string-mode pin, tenant-id refusal (incl. the `\!` payload), header-comment containment, backslash/quote round-trip through the dump, and timestamp-lookalike text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(export): round-2 review — one stale example, two overstated comments Second review pass found no defect in the four fixes; the runtime behaviour holds. Three accuracy items: - The SqlDumpOptions doc block still showed `-v tenant_id=default`, the slug that cannot work. Every other copy was corrected in 2f4df82; this one was missed. - Two comments claimed the `?tenantId=` value reaches a psql `\set`. It doesn't: literal mode emits no meta-commands at all. The real exposure is the header comment in a file psql executes, which is why the value is validated rather than escaped — now stated accurately. - Documented that the portable form needs psql 10+ (`\if`), and that `?tenantId=` is the escape hatch for older clients, since an old psql would skip the conditional and apply the default over a supplied -v. Also switched the empty-bundle test off the slug so no test models a restore command that would fail. Reviewer confirmed against the source: stripping CR/LF is sufficient for psql's lexer (only CR and LF end a line — U+0085/U+2028/U+2029 and form feed stay inside the comment); `SET LOCAL` is correctly placed after BEGIN and applies to the literals parsed after it; and knex's columnInfo returns `timestamp with time zone`, which `isTimestampType` matches, so no timestamp column stopped being revived. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(export): round-3 review — 400 on a malformed tenantId, and honest gap docs Third pass found no defect in the restore mechanics; both remaining items are contract accuracy. 1. `?tenantId=a&tenantId=b` parses as an ARRAY, and "present but not a string" was treated as absent — so a malformed request quietly got a portable 200 dump instead of the promised 400. Present-and-not-a-scalar is now a 400, like every other malformed value. Covered by the first route-level test this endpoint has had; previously either route's validation could have been deleted without failing the suite. 2. DOCUMENTED PROMISES THAT THE CODE DOESN'T KEEP. The review found `docs/payout-funding.md` §4 claiming the hosted funding sidecars are exportable, and `docs/white-label-custom-domains.md` claiming `Tenant.whiteLabel` exports losslessly — neither table is in the export set. `ARCHITECTURE.md` also still said every table imports with `onConflict('id')`, which stopped being true when PartnerCommission (keyed on partnerId) was added. Rather than bolt six tables on, `docs/data-portability.md` now states plainly what is NOT exported and why. `PayoutReversal` is called out as the material one: payout status is DERIVED from it, so a restored database can hold a `reversed` payout with no ledger behind it. Exporting those needs an FK decision first — `HostedFundingAuthorization.adminId` references `Admin`, which is deliberately never exported — and that is its own PR. A documented gap is recoverable; a promise the code doesn't keep is what makes an export untrustworthy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(export): round-4 review — make every export claim in the repo true No runtime defect found; the whole finding was that the contract still contradicted itself, including in the file I corrected last round. - `data-portability.md` opened with "every table exports" and then listed the exclusions. It now says what it means: everything IN THE BUNDLE exports, and here is what isn't in it. - It also called the bundle a complete "commission/payout ledger" three lines after admitting `PayoutReversal` is missing — and a reversal ledger is precisely part of the payout ledger. Now scoped honestly: a complete record of attribution, commissions and payouts, but NOT of payout corrections. - The never-exported list was incomplete (`Admin`, `SignupBlocklist`, `PlatformAuditLog`, `PlatformSession*`) and `WebhookEndpoint` — the customer's own outbound config, as opposed to its delivery log — was missing from the gap list entirely. - `payout-funding.md`, `white-label-custom-domains.md` and the HostedFunding type comments all still asserted these tables export today. They describe a design intent; they now say so and point at the gap list. - The portal promised "Download everything, any time." Tests: the valid-tenantId assertion checked only a 200 and `BEGIN;`, which would also pass if the parameter were ignored — it now asserts the id is actually baked in and no psql variable remains. Added the missing per-table route case and an assertion on the error body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test): escape the backslash in the psql-variable assertion Lint caught a useless-escape in the assertion added by the previous commit: '\set' in a TS string is just 'set', so the test was asserting the dump doesn't contain "set tenant_id" rather than "\set tenant_id". It passed either way, which is why it slipped through. My verification chain also let it slip: `pnpm lint | tail && git commit` takes the pipeline's exit status from `tail`, so a failing lint didn't stop the commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(export): round-5 review — the last stale export claims, and a real cross-tenant test No runtime defect again; the review confirmed the table categorisation is exhaustive (38 in TABLES = 14 exported + 13 never + 11 known gaps) and that round 4 closed all three holes it had found. What was left was language I hadn't touched, in the places furthest from the code I was editing: - `README.md` still said "export everything" and "every table" - `ARCHITECTURE.md` said the migration is "the six raw + derived tables" - `export.ts`'s own header said ANY export re-imports — but CSV is a view, not a restore format, which the doc already said - the hosted-funding and white-label MIGRATION comments, and the `TenantRow.whiteLabel` type comment, all describe design intent in the present tense `CLAUDE.md` §2 is deliberately left alone: "every table must be exportable" is the project's stated commitment, not a claim about today's implementation. Weakening the principle to match the code would be the wrong direction — docs/data-portability.md records the delta instead. Also added the cross-tenant restore test the review kept flagging as missing. Every existing round-trip exports and imports under the SAME tenant, so all of them would pass with the tenantId rewrite deleted — the one behaviour that makes hosted → self-host work. Writing it surfaced a property worth recording: primary keys are global, not per-tenant, so a bundle cannot be restored alongside its source in one database (`onConflict(pk).ignore()` sees the originals). The real scenario is a different database, which the test now models. Raised the timeout on the DB-heavy round-trip tests: several were running within a second of vitest's 5s default, and when one timed out it left rows behind that failed the next test. Flaky money-adjacent tests are worse than slow ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(export): scalars are JSON too, and scoping no longer rests on RLS alone Two round-6 findings, both verified against a live database. Scalar and null JSON did not round-trip. Both serializers re-serialized a json/jsonb value only when `typeof v === 'object'`, but pg parses json/jsonb through JSON.parse — so a column holding '"hello"' hands back the JS string `hello` and one holding '42' hands back a number. Those were emitted as the SQL literals 'hello' and 42, which the column rejects, so a perfectly valid stored value broke the restore outright. Any non-null JSON value is now re-serialized regardless of its JS type. Confirmed empirically first: jsonb_typeof reports `string`/`null` for these, and a NOT NULL column accepts both. Export scoping was the one path that broke the codebase's own safety net. `exportTable` was an unfiltered `select *` relying entirely on RLS, but appDb falls back to the privileged DATABASE_URL when DATABASE_URL_APP is unset — which db.ts documents as supported precisely because "app-level tenantId filtering still applies". It did not apply here. exportTable now takes an explicit tenantId; every exportable table carries one, so the filter is complete. Prod is unaffected (DATABASE_URL_APP is set there); the exposure was multi-tenant self-host without the app role. Also strengthens the v1-bundle test, which called importBundle directly and supplied no schemaVersion — so it would have stayed green if v1 support were dropped from the HTTP route, which is where the version gate actually lives. It now imports through the route and checks an unsupported version is still refused. All three fixes verified to fail their tests when reverted (the JSON ones with "invalid input syntax for type json"). Residual documented: JSON null and SQL NULL are indistinguishable after `select *`. 270 tests green, typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth): bind an API key to its tenant, not to RLS Round 7 on the export work found that the tenant FILTER I added does not fix what I implied it fixed. requireAuth matched a key on prefix + hash with no tenant predicate and leaned on RLS to keep the lookup inside the tenant. That holds on the app role — but appDb falls back to the privileged DATABASE_URL when DATABASE_URL_APP is unset, a configuration db.ts documents as supported, and there RLS is bypassed. Tenant A's admin key then authenticated against /t/tenant-b/..., and every tenant-scoped query below it — including my newly-filtered export — faithfully served B's data. ApiKey carries tenantId, so the lookup is now bound to the request's tenant exactly. This is broader than export: it applies to every authenticated route in that configuration. Prod is unaffected (DATABASE_URL_APP is set). Verified by reverting: the foreign key authenticates and receives a 200 with another tenant's full export. Also corrects two claims of mine: - docs said JSON-null-vs-SQL-NULL was the ONLY residual. It is not: pg parses jsonb through JSON.parse, so 9007199254740993 restores rounded and 1e400 becomes Infinity, which JSON.stringify writes as null. Silent, unlike the null case. Both residuals are now documented and covered by tests that assert the CURRENT lossy behaviour, so making the parse lossless later fails those tests rather than leaving the doc stale. - the JSONB-number import test passed with its fix reverted: pg prepares a JS number as the text "42", which is already valid JSON input, so the old guard was never exercised. Switched to a string, which is prepared bare and rejected. 273 tests green, typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth): bind the session cookie to its tenant too — round 7 fixed half Round 7 bound the API-key path and I called the hole closed. It was not: resolveSession matched on {prefix, tokenHash} with no tenant predicate and leaned on the same RLS that the privileged-pool fallback bypasses. So on a deployment with DATABASE_URL_APP unset, tenant A's admin cookie authenticated against /t/tenant-b/... and resolvePrincipal handed back an admin principal for B — the identical hole, in the other credential path. resolveSession now takes `expectTenantId` as a REQUIRED parameter. Required rather than optional deliberately: the failure mode here is a security predicate someone forgets to pass, and an optional one invites exactly that. `/session/home` passes an explicit null because it is deliberately tenant-discovering and never grants a principal from the result. The Admin/Partner revocation lookups are scoped to the session's tenant as well — on the privileged pool a bare principalId would match another tenant's row. resolvePrincipal now throws when req.tenantId is missing rather than defaulting to null, since null is resolveSession's any-tenant escape hatch. Verified by reverting: the foreign session authenticates and receives a 200 with another tenant's full export. 273 tests green, typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth): signout must revoke the token it was given — round 8 broke logout Round 8 routed signout through resolveSession, which it had just bound to the request tenant. That silently broke logout in a real flow: `op_session` is host-wide at path '/', so entering a second workspace overwrites it, and a signout from a stale tab carries tenant B's token to tenant A's URL. The tenant-filtered lookup found nothing, the route cleared the browser cookie and returned 200 — and B's session stayed valid server-side. A client that kept the token could keep using it. "Logged out" that leaves a live token is worse than the cross-tenant read the binding was added to prevent. Signout now uses revokeSessionByToken: an exact-token revocation that resolves no principal and applies no tenant predicate. Revoking a token the caller already holds grants nothing and leaks nothing, so it needs no binding — it is strictly destruction of the bearer's own credential. It is a separate function rather than a flag on resolveSession precisely because the two operations have different safety requirements, and one function serving both is how the binding got applied where it did not belong. Verified by reverting: the session survives "logging out". Unrelated observation while running this: the API suite is mildly flaky under parallel files sharing one Postgres — two runs failed different files (export-roundtrip once, compound-rules once) and two consecutive runs then passed 276/276. Pre-existing, not introduced here, but worth knowing before trusting a single red run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: handoff brief for the remaining payment-audit work (#10, #12, #8) Self-contained brief for a fresh agent to pick up the three deferred items: the direct-Connect transfer double-pay (durable-intent restructure, with the funding executor as the reference and the deterministic-key shortcut warning), the rest of the funding-pipeline races, and export portability. Includes the ground truth (prod facts, manual-payout context, commands, branch conventions) and the 12 shipped audit PRs + post-merge actions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: the three remaining audit items are shipped (#73, #74, #75) Turns the handoff brief into a status record: what each item was, what actually shipped, and — the part that is still open — the staging exercises that have to pass before either money path is trusted. Item A (#10, PR #73): planner/executor split with a durable payout intent and a frozen commission set. Item B (#12, PR #75): the three funding races plus a live-Stripe backstop for missed refund/reversal webhooks. Item C (#8, PR #74): the three missing tables, per-table primary keys, a portable SQL dump, and two array round-trip bugs the test found. No code left on any of the three; the remaining work is the staging checklists in docs/direct-connect-payouts.md and section H of the funding staging runbook, plus the two post-merge prod actions for #62 and #63. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: bring the handoff up to date with five rounds of review The doc still said "all three items are implemented, what's left is staging" — written before any adversarial review ran. Since then each branch has gained six review commits, two of the three original implementations turned out to contain real double-pay paths, and several of the FIXES did too. Rewritten around what a fresh agent actually needs: - the three PRs, what each contains now, and that nothing is merged; - the pattern, stated up front: every round found defects in the previous round's fix, two coverage mechanisms looked like rotation without being it, and three tests passed with their own fix reverted. In this code a fix that looks like the property is not the property; - what must happen before merge — #74 after #62 (PartnerProgram has no RLS on main and exports rely on it), #73 alongside #71, and the two staging matrices that are the actual gate; - the load-bearing invariants, so they don't get casually broken: the epoch fencing, the two clocks, the lease-vs-request-budget relationship, the ambiguous-error classification, the commit-after-work cursor, the inbox lease answering 409; - the known gaps as decisions with reasons, not an apology list. Also records the operational trap that cost me a bad commit: piping lint into `tail` takes the exit status from `tail`, so a failing lint does not stop the commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: correct the #71 pairing and record what actually blocks staging Re-verified the handoff against the branches rather than trusting it. The merge-order constraint "#73 needs #71 alongside it" was misattributed. #71 fixes funding/executor.ts — the hosted-funding rail, which is #75's file, not #73's. #73 never touches it (only funding/interlocks.ts) and its own reconcile at payout-transfers.ts:609 already paginates. #75 still carries main's unpaginated limit:100 at funding/executor.ts:458, so #75 without #71 ships the duplicate-transfer hole #75 exists to close. Verified by extracting the merged blob and reading it — a clean merge-tree proves no textual conflict, not that the fix survived. It does survive, in either order. All file-sharing pairs merge clean. Also records the thing the doc called "the actual gate" but never scoped: there is no openpartner staging app. Building one isn't required — the runbook allows `stripe listen`, and local tooling is ready — so the gate is a Stripe test-mode key plus the runbook fixtures, not infrastructure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: #66 was red, merging is blocked by an unsatisfiable rule, migrations do auto-run Findings from re-verifying the handoff against the repo rather than reading it. #66 had been failing typecheck+test since 08-08 behind a real defect: the negative-age guard was `ageMs < 0`, but Event.ts comes from Stripe's whole-SECOND `created`, so a same-second conversion truncates to before its click and was dropped silently. Fixed on the branch with a 5-minute skew grace plus a test verified to fail when the grace is reverted. main requires an approving code-owner review that a sole maintainer can never satisfy — the likeliest reason 16 PRs have sat open. enforce_admins is off, so --admin works. Each PR is MERGEABLE against main yet two collide with the batch (#66x#61/#64 on integration.test.ts, #70x#69 on .env.example). Recorded with resolutions. Full 11-PR combination validated: typecheck + lint clean, 292 tests green. Corrected two load-bearing facts: prod migrations DO run on api boot (the entrypoint has done so since April, and the July incidents were code/schema mismatches, not drift), and the Stripe test key already exists in the repo-root .env. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: the staging gate is no longer hypothetical — both matrices have run #73's six scenarios: 37 assertions, 0 failures. #75's H1/H5/H6/H7/H8/H9 (+H11 incidentally): 21 assertions, 0 failures. H2, H3, H4, H10 and H12 are still unrun and the doc says so rather than implying the section is done. No staging server was needed. "Staging" here means an instance talking to test-mode Stripe, not a deployed environment, so both ran locally against Postgres in Docker. Records the reusable test-mode fixtures (platform account, onboarded and un-onboarded Connect destinations, the ACH customer + mandate, and how to top up platform balance) so nobody rebuilds them. The finding that justifies having run this at all is about Stripe rather than us: paymentIntents.search is eventually consistent, so a retry firing inside the indexing window will not find an intent that exists. The frozen idempotency key is the load-bearing defence inside 24h and search is the fallback past it — assumed by the design, now confirmed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: round 6 — ten defects fixed, and the assumption round 5 rested on Round 5's fix assumed POST_COOLDOWN_MS could bound a Stripe request. It cannot: stripe-node's timeout is socket-inactivity, resetting after each request stage, so a slow-but-progressing POST has no wall-clock bound. Both money rails now treat "I did not see it" as UNKNOWN rather than ABSENT — the transfer executor no longer re-arms on elapsed time, and an empty paymentIntents.search no longer frees allocations. That trades liveness for safety, so both rails gained operator disposition. The doc previously called operator tooling the most valuable follow-up; it is now a prerequisite, because a hold with no release is a leak. Records that four sweep-coverage mechanisms have now looked like rotation without being it, and adds the lesson the round-6 revert checks taught: three of my OWN tests passed with their fix reverted — a lock test where blocking was not the property, a coverage test with no churn, and a staging scenario that passed when it never reached Stripe. Running the suite is not the check; reverting the fix and watching the test fail is the check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: round 7 — twelve more, nine of them in round 6's fixes Round 6 predicted that its changes were "mostly deletions, so less new surface to breed a round 7". That was wrong in a checkable direction and the doc now says so: the deletions produced no findings at all, and every serious round-7 finding is in something round 6 ADDED — the operator functions, the webhook fallback, the retry set. The transferable lesson is that the dangerous change here is the one that adds a mechanism, and "I removed unsound logic" is not grounds to expect a clean next round. Also extends the weak-test checklist with the four shapes round 7 caught: an early guard short-circuiting before the code under test, a cursor that wraps and covers everything by accident, a value that is already valid so the guard is never exercised, and array indexing where order is not guaranteed. Plus the one from round 7's own fixes — a lock test that asserts "it blocked" proves nothing, because it will block eventually at some later write; assert the value the lock protects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: rounds 8 and 9, the ten OPEN findings, and the decision that gates them Round 8 is fixed and pushed. Round 9 is NOT: ten of its eleven findings are open, and the Codex output they came from lived in a session scratchpad that no longer exists — so they are reproduced in full in a new §0 at the top, which is where a new agent should start. The most important part is a correction to my own reasoning. I claimed the verify-then-write gap between Stripe and Postgres meant those operations could not be made safe, and I acted on that. It is wrong: the target is CONVERGENCE, not atomicity, and this codebase already works that way — the finalizer's own comment says the guarantee is that the reversal webhook can always find the payout. resolveDuplicateReview simply is not wired into that mechanism: the webhook fallback accepts only 'posted' and 'reconcile_required', so a reversal during duplicate_review is dropped and the payout is recorded paid with the money gone. That makes the highest-value outstanding change a one-predicate widening, not a rewrite. Also records the open product question that determines the rest — when a payout freezes, must someone unfreeze it quickly, or can it wait for an engineer — and Codex's answer that this, not a technical fact, is what decides between deleting the operator surface and rebuilding it as durable intents. Plus its round-10 estimates and the line worth carrying: review count is not the same metric as operational safety. Fewer findings from deleted code is not safety. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: round 10 — the decision is B, all ten round-9 findings closed Keith answered §0.4: fast unfreeze, so B (durable operator-recovery requests). Round 10 fixed all ten round-9 findings across #73/#75, then a same-day Codex pass refuted six of seven claims about those fixes; its thirteen fixable findings are fixed and pushed, and the two that remain are the prove-absence limit — now with a transfer.created alarm and B's request-ledger tombstone as the close. §0 rewritten: what happened, the two findings that remain by design, the ordered next steps (merge first — the PRs land squashed, so B stacks on main, not on them), and B's full design ready to execute. #75 now adds a migration; the ground truth and PR notes say so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: the batch is merged — all sixteen PRs on main Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…t §0.4) + round-11 fixes (#76) * feat(recovery): durable operator-recovery requests — decision B (audit handoff §0.4) Operators insert an append-only OperatorRecoveryRequest; the existing scheduler machinery applies it by calling the four operator functions under their own fences. The request row is the tombstone for the prove-absence limit: who accepted the risk, when, on what evidence. - OperatorRecoveryRequest table (RLS'd operational sidecar, listed with the export gaps), TABLES entry + row types + app-role grant - applyRecoveryRequests: skip-locked lease claim (DB clock both sides, token-fenced writes), tenant boundary check before anything else, outcome mapping (success → applied; definitive → refused; retryable trio stays pending, paced at 15m, capped at 10 attempts → failed + alert); wired at the top of the payout-transfers job and the funding collector tick so a released intent executes on the same tick - §0.2 prove-absence close: applied direct-Connect requests get a 24h transfer-group recheck (detector only) that alarms on a live transfer the payout no longer expects, even if the transfer.created webhook was missed - Admin API: POST /payouts/:id/recovery, POST /funding/batches/:id/recovery (durable insert + one inline apply, synchronous verdict), GET /recovery-requests - transfer.created added to the platform family map (registered on Destination A 2026-08-14; was unconstrained) 507 tests green (48 files), typecheck + lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: record §0.3 status — webhook cutover done, B built, auto-approve still unverified Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(recovery): round-11 hardening — takeover truthfulness, poison-proof claim, append-only grant, fenced alarms Codex round-11 refutation pass (4 of 6 findings confirmed, 2 accepted as outside the trust boundary / by-design with an alert added): - A claim that dies between its action committing and its settle no longer yields a lying 'refused' row: a takeover of an EXPIRED lease treats definitive refusals with suspicion, re-reads the target for THIS request's operator marker (req_<id> in lastError/failureReason), and settles applied:by_interrupted_attempt when found — else refuses annotated as ambiguous, with an alert. Claim is now select-then-update in one transaction so the old leaseAt (the takeover signal) survives. - A hand-poisoned attempts counter at int4 max made the claim STATEMENT raise on every pass — at the top of the job that moves money. The increment is clamped, and claim/recheck failures are caught so the rail always runs. - The app-role grant narrows to SELECT + INSERT: every update runs on the privileged pool, so append-only becomes a database property, not a convention. ensure-app-role revokes the wider grant on provision. - Recheck alarms now emit only AFTER the token-fenced write wins, so a stale pass that lost its lease mid-listing cannot re-raise a settled alarm; recheckOne is side-effect-free. - Pending requests on an instance with no Stripe client now alert every pass instead of holding silently. 510 tests green (3 new), typecheck + lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…sion B) The raw function calls stay documented as the break-glass path — the API is the supported one: durable insert, inline verdict, scheduler retries, audit tombstone, 24h group re-check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… — all clear H2/H3/H4/H10/H12 (23 assertions, 0 failures, 2026-08-14): - H2: past the window the retry resolves purely by search-adopt, with create FORBIDDEN (test clocks cannot move key retention — documented) - H3: a create failing WITH an intent stamps the id from the error and the retry confirms that PI (fbpc: key), never a second create - H4: release vs in-flight create as two genuinely interleaved flows — orphan canceled, held on empty-is-not-absent, full release on resume - H10: release halted on search-down holds safely; the resume landed on the payment-wins path with the charge id stamped (H11 verified again) - H12: a REAL refund through the signed webhook route froze the batch mid-executor-run; exactly one transfer left, executor stopped Adds STAGING_SCENARIOS=<subset> filtering. Runbook + handoff doc updated; only a true multi-process lease race remains unexercised. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ge the applied verdict The audit marker and the operator-controlled reason land in the same lastError/failureReason string, so a substring match let request B smuggle req_<A> inside its reason and settle request A 'applied' on takeover. The check now matches the per-kind stamped prefix from the START of the string; everything before the marker is authentication-derived, so the reason tail cannot reach it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…c grant swap, repaired counters, discriminating staging assertions Codex round-12 refutation pass over everything its round-11 pass never saw (the round-11 fixes, the marker anchoring, the staging scenarios). Five findings; four fixed, one accepted + documented: - requestedBy could embed another request's marker prefix (schema-level text, app-role insertable): the operator string now sanitizes / and : out of the name segment at stamp time, so the first /req_ in any stamp is the stamping request's own delimiter. Tested through the REAL apply path — a malicious request genuinely applies, the victim must refuse. - ensure-app-role's revoke-then-grant now runs in one transaction — a boot killed between the statements can no longer strand the app role with no privileges. - Negative poisoned counters (least() kept them negative → cap false for ~2^31 claims) are repaired with greatest(...,0)+1 on both counters. - Accepted: a crash between the recheck's fenced commit and its log line loses the LINE, not the finding — recheckOutcome durably records the orphan and transfer.created stays the primary alarm; documented in place rather than building an alarm outbox. - All five staging scenarios sharpened so their named regressions fail: H2/H3 assert batch PROGRESS, H4 follows through to a decisive landing, H10 fails if stuck with a terminal PI after a full window of ticks (single-tick probes are invalid — search is not monotonic per request, observed live), H12 gains a control batch proving both partners are paid un-frozen. Re-run against real Stripe: 29 assertions, 0 failures. 513 unit tests green, typecheck + lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New harness (staging-two-process-races.ts) spawns genuinely concurrent OS processes with separate DB connections against real Stripe test mode: - A: two executors race one FRESH intent end-to-end — exactly one transfer at Stripe, payout paid once, commission claimed once - A2: reconcile+finalize duel over a held intent whose transfer already exists — finalized once against the found transfer, never re-posted - B: 40 inbox events claimed under real contention (19/21 split) — no event claimed by both processes, all stamped processed - C: 24 pending recovery requests split across two apply loops (4/20) — every request settled applied with attempts === 1 17 assertions, 0 failures. Also: auto-approve backlog VERIFIED cleared by a read-only query against openpartner_prod (zero mature-unswept accrued commissions) — §0.3 item 2 closed with direct evidence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gitimate-landing acceptance Codex round-13 pass over the round-12 fixes (5 findings; 4 fixed, 1 accepted with evidence): - HIGH fixed: request ids are schema-unconstrained, so id = <A's id>+'X' produced a stamp of which A's suffix-less marker (rearm / kept / all_reversed) was a strict prefix — those kinds now match by EXACT equality (their full stamp is known and never truncated); the reason-carrying kinds keep the prefix form, whose ':' after the id already terminates it. Test drives the full interleaving via the real apply paths. - MEDIUM accepted: pre-sanitizer stamps would mismatch — but prod has ZERO recovery-request rows ever (verified), and the mismatch lands on the safe annotated-refusal branch. Documented; no legacy fallback on purpose (it would re-open the round-12 forgery). - 3 LOW fixed in the staging script: H2 accepts funding_failed when the adopted PI is already canceled (release protocol owns it); H3 accepts a real confirm rejection with the attempt recorded; H4/H10 replace timing-sensitive checks with a DETERMINISTICALLY WARM search probe — a client whose search always returns the PI — so a live resume must act on that tick, separating cold search from a dead resume without racing Stripe's non-monotonic index. Staging re-run (H2/H3/H4/H10): 20 assertions, 0 failures. 514 unit tests green, typecheck + lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rs, worker-mode guards, rendezvous barrier Codex round-14 pass over the two unreviewed commits (the two-process harness + the round-13 fixes). Ten findings; nine fixed, one documented: - HIGH: non-ULID request ids are refused at the door (invalid_request:id) — closes the whole id-shaped marker-forgery class (+X extension, :B colon injection, anything varchar(32) admits); exact/prefix matching stays as defense-in-depth. - HIGH: a takeover-ANNOTATED refusal now schedules the §0.2 group recheck exactly like an applied request — a successful rearm's marker is legitimately cleared when the executor finalizes, so the takeover cannot tell 'never acted' from 'acted and completed', and the superseded generation's in-flight POST is precisely the late-lander the recheck exists for. Ambiguity earns the backstop. - HIGH: the harness guards (test key, local DB) now run in WORKER mode too — RACE_WORKER=intent against a prod URL refused nothing before. - MEDIUM: warmSearchStripe honors the query (blind stubs mask query-scoping regressions); H3 retrieves the PI to distinguish a genuine confirm rejection from confirm-ok-but-CAS-failed; reset() deletes Identity before Click (non-cascading FK made a half-wipe possible). - LOW: workers rendezvous on ready-files (cold-start skew cannot hand one process the whole workload); Race A accepts the safely-posted slow-Stripe landing; balance-aware top-up + scratch-file cleanup. - Documented: the harness's honest scope — money/claim exactly-once under real concurrency; internal fence discrimination is owned by the unit suites with staged seams. Re-runs: two-process 17/17, H3/H4/H10 15/15, unit suite 515 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uery probes, deterministic Race A completion Codex round-15 pass on the round-14 fix commit (3 findings, all fixed): - The tenant-boundary check (and its ALERT) now runs before the id-shape gate: a hand-insert that was both malformed AND cross-tenant used to settle quietly as invalid_request:id, suppressing the loud tenant_mismatch alert. Boundary problems refuse loudly; shape problems refuse quietly. The id gate still precedes every operator-function call and marker use. - warmSearchStripe matches the EXACT production query (metadata key + batch id, the shape findFundingPaymentIntent builds) — a substring check still certified a wrong-metadata-key regression. - Race A no longer accepts the safely-posted landing as terminal: it completes the intent through the real finalizer with the listed transfer and holds the full paid-once assertions either way, so a broken live finalization can no longer false-pass. Re-runs: two-process 17/17, H4/H10 7/7, unit suite 516 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
This PR exists purely as an ultrareview target — it represents everything on main that hasn't been reviewed since the last pass.
Scope
14 commits between `532d7c0` (last reviewed regression tests) and `HEAD`:
Plus three small CI fixes (`63f016d`, `ba67b70`, `a6db3a6`).
Areas of highest concern for review
Not for merge.