diff --git a/.github/workflows/ci-integration.yml b/.github/workflows/ci-integration.yml index 032b90bae..83f8e9fbe 100644 --- a/.github/workflows/ci-integration.yml +++ b/.github/workflows/ci-integration.yml @@ -59,6 +59,8 @@ jobs: run: bun run cms:migrate:status - name: Apply demo seed run: psql "$DATABASE_URL" -f supabase/seed.sql -v ON_ERROR_STOP=1 + - name: Verify donation fee replay + run: psql "$DATABASE_URL" -f tests/integration/supabase/donation-fee-replay-verification.sql -v ON_ERROR_STOP=1 - name: Verify seed counts run: | profile_count=$(psql "$DATABASE_URL" -t -A -c "SELECT COUNT(*) FROM public.profiles;" | tr -d '[:space:]') diff --git a/.github/workflows/qa-smoke-preview-deploy.yml b/.github/workflows/qa-smoke-preview-deploy.yml index a2ed8ab59..349edc69e 100644 --- a/.github/workflows/qa-smoke-preview-deploy.yml +++ b/.github/workflows/qa-smoke-preview-deploy.yml @@ -141,8 +141,8 @@ jobs: if: steps.scope.outputs.any == 'true' env: CLAUDE_QA_ROUTINE_WEBHOOK_URL: ${{ secrets.CLAUDE_QA_ROUTINE_WEBHOOK_URL }} - QA_TEST_EMAIL: ${{ secrets.QA_TEST_EMAIL }} - QA_TEST_PASSWORD: ${{ secrets.QA_TEST_PASSWORD }} + QA_TEST_EMAIL: ${{ secrets.QA_PREVIEW_TEST_EMAIL && secrets.QA_PREVIEW_TEST_PASSWORD && secrets.QA_PREVIEW_TEST_EMAIL || secrets.QA_TEST_EMAIL }} + QA_TEST_PASSWORD: ${{ secrets.QA_PREVIEW_TEST_EMAIL && secrets.QA_PREVIEW_TEST_PASSWORD && secrets.QA_PREVIEW_TEST_PASSWORD || secrets.QA_TEST_PASSWORD }} VERCEL_ADMIN_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_ADMIN_AUTOMATION_BYPASS_SECRET }} VERCEL_ADMIN_PROJECT_ID: ${{ secrets.VERCEL_ADMIN_PROJECT_ID }} VERCEL_DONOR_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_DONOR_AUTOMATION_BYPASS_SECRET }} @@ -175,11 +175,12 @@ jobs: - name: Validate required preview smoke secrets if: steps.scope.outputs.any == 'true' env: + QA_PREVIEW_CREDENTIALS_PARTIAL: ${{ (secrets.QA_PREVIEW_TEST_EMAIL != '' && secrets.QA_PREVIEW_TEST_PASSWORD == '') || (secrets.QA_PREVIEW_TEST_EMAIL == '' && secrets.QA_PREVIEW_TEST_PASSWORD != '') }} ADMIN_AFFECTED: ${{ steps.scope.outputs.admin }} DONOR_AFFECTED: ${{ steps.scope.outputs.donor }} MISSIONARY_AFFECTED: ${{ steps.scope.outputs.missionary }} - QA_TEST_EMAIL: ${{ secrets.QA_TEST_EMAIL }} - QA_TEST_PASSWORD: ${{ secrets.QA_TEST_PASSWORD }} + QA_TEST_EMAIL: ${{ secrets.QA_PREVIEW_TEST_EMAIL && secrets.QA_PREVIEW_TEST_PASSWORD && secrets.QA_PREVIEW_TEST_EMAIL || secrets.QA_TEST_EMAIL }} + QA_TEST_PASSWORD: ${{ secrets.QA_PREVIEW_TEST_EMAIL && secrets.QA_PREVIEW_TEST_PASSWORD && secrets.QA_PREVIEW_TEST_PASSWORD || secrets.QA_TEST_PASSWORD }} VERCEL_ADMIN_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_ADMIN_AUTOMATION_BYPASS_SECRET }} VERCEL_ADMIN_PROJECT_ID: ${{ secrets.VERCEL_ADMIN_PROJECT_ID }} VERCEL_DONOR_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_DONOR_AUTOMATION_BYPASS_SECRET }} @@ -192,6 +193,11 @@ jobs: run: | set -euo pipefail + if [[ "${QA_PREVIEW_CREDENTIALS_PARTIAL}" == "true" ]]; then + echo "::error::Both QA_PREVIEW_TEST_EMAIL and QA_PREVIEW_TEST_PASSWORD must be configured together." + exit 1 + fi + missing=() require_secret() { @@ -434,8 +440,8 @@ jobs: QA_ADMIN_BASE_URL: ${{ steps.deploy_admin.outputs.url }} QA_DONOR_BASE_URL: ${{ steps.deploy_donor.outputs.url }} QA_MISSIONARY_BASE_URL: ${{ steps.deploy_missionary.outputs.url }} - QA_TEST_EMAIL: ${{ secrets.QA_TEST_EMAIL }} - QA_TEST_PASSWORD: ${{ secrets.QA_TEST_PASSWORD }} + QA_TEST_EMAIL: ${{ secrets.QA_PREVIEW_TEST_EMAIL && secrets.QA_PREVIEW_TEST_PASSWORD && secrets.QA_PREVIEW_TEST_EMAIL || secrets.QA_TEST_EMAIL }} + QA_TEST_PASSWORD: ${{ secrets.QA_PREVIEW_TEST_EMAIL && secrets.QA_PREVIEW_TEST_PASSWORD && secrets.QA_PREVIEW_TEST_PASSWORD || secrets.QA_TEST_PASSWORD }} VERCEL_ADMIN_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_ADMIN_AUTOMATION_BYPASS_SECRET }} VERCEL_DONOR_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_DONOR_AUTOMATION_BYPASS_SECRET }} VERCEL_MISSIONARY_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_MISSIONARY_AUTOMATION_BYPASS_SECRET }} diff --git a/README.md b/README.md index 9191067f6..4d92d72df 100644 --- a/README.md +++ b/README.md @@ -243,6 +243,8 @@ This repo uses **Bun** pinned in root `package.json` `packageManager` and `.bun- ### Monorepo Workspace Contract +Vercel installs with `bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile` to keep the build-image package manager on the workspace pin. See [CI toolchain guidance](docs/ci.md#bun-toolchain). + Bun workspaces + Turborepo: ```text diff --git a/apps/admin/vercel.json b/apps/admin/vercel.json index dd9341380..9667a43d1 100644 --- a/apps/admin/vercel.json +++ b/apps/admin/vercel.json @@ -1,6 +1,6 @@ { "$schema": "https://openapi.vercel.sh/vercel.json", - "installCommand": "bun install --cwd ../.. --frozen-lockfile", + "installCommand": "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", "buildCommand": "cd ../.. && bun run build:admin", "ignoreCommand": "node ../../scripts/vercel/should-ignore-build.mjs admin", "git": { diff --git a/apps/donor/vercel.json b/apps/donor/vercel.json index 4d5a1e1d2..444e9db35 100644 --- a/apps/donor/vercel.json +++ b/apps/donor/vercel.json @@ -1,6 +1,6 @@ { "$schema": "https://openapi.vercel.sh/vercel.json", - "installCommand": "bun install --cwd ../.. --frozen-lockfile", + "installCommand": "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", "buildCommand": "cd ../.. && bun run build:donor", "ignoreCommand": "node ../../scripts/vercel/should-ignore-build.mjs donor", "git": { diff --git a/apps/missionary/vercel.json b/apps/missionary/vercel.json index 298823ce9..1410a633d 100644 --- a/apps/missionary/vercel.json +++ b/apps/missionary/vercel.json @@ -1,6 +1,6 @@ { "$schema": "https://openapi.vercel.sh/vercel.json", - "installCommand": "bun install --cwd ../.. --frozen-lockfile", + "installCommand": "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", "buildCommand": "cd ../.. && bun run build:missionary", "ignoreCommand": "node ../../scripts/vercel/should-ignore-build.mjs missionary", "git": { diff --git a/docs/ci.md b/docs/ci.md index 7feb84f42..0dcc8d0a0 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -34,7 +34,7 @@ The exact live required-check sets are recorded only in § Branch protection. - **Pinned version:** root `package.json` `packageManager` and `.bun-version` (currently `bun@1.4.0`, stable only — never canary). - **Runtime vs package manager:** Bun is the install/script runner. Next.js apps still execute on Node.js (Vercel project `nodeVersion` is `24.x`). Do not pass `bun --bun`, and do not set `bunVersion` in `apps/*/vercel.json`. - **Vercel Functions Bun 1.4 is a separate runtime:** [Vercel's Bun 1.4 changelog](https://vercel.com/changelog/bun-1-4-is-now-available-in-vercel-functions) documents opting **Functions and Middleware** onto Bun via `"bunVersion": "1.4.x"`. That is not how you pin the package manager. `"1.x"` still selects Bun 1.3.14 on Functions. Next.js on the Bun runtime also requires `bun run --bun next dev|build` ([runtime docs](https://vercel.com/docs/functions/runtimes/bun)). Core stays on the Node path (`next dev` / `next build` / `next start`) because Payload, Stripe, Supabase SSR, and eve-runtime are validated there; Vercel treats the Bun Functions runtime as an explicit breaking-change opt-in. -- **Vercel install vs GitHub install:** App `installCommand` is `bun install --cwd ../.. --frozen-lockfile` (workspace root, frozen lockfile). That matches [Vercel package-manager detection](https://vercel.com/docs/package-managers) for `bun.lock` (`bun install`, not `bun ci`, and not `bun install --save-text-lockfile`). GitHub Actions keeps `bun ci --no-cache --backend=copyfile` for the portable file-copy backend. [Pinning a Bun version for Vercel _builds_](https://vercel.com/kb/guide/how-to-pin-a-specific-bun-version-for-vercel-builds) is `bunx bun@1.4.0 install`; Corepack does **not** pin Bun (it is for pnpm/Yarn). Do not change the install command unless a deploy proves the build-image Bun cannot read this `lockfileVersion` 1 file. +- **Vercel install vs GitHub install:** App `installCommand` is `bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile` (workspace root, pinned package manager, frozen lockfile), following [Vercel's documented build pin](https://vercel.com/kb/guide/how-to-pin-a-specific-bun-version-for-vercel-builds). Preview deployment `dpl_8p7c7tAFVzvdLZY5t4FB8NuTxdCx` proved that the build image's Bun 1.3.14 cannot install the current lockfile without drift. Pinning installation does not opt Functions into the Bun runtime. GitHub Actions keeps `bun ci --no-cache --backend=copyfile` for the portable file-copy backend. Corepack does **not** pin Bun (it is for pnpm/Yarn). - **GitHub Actions:** `ci.yml`, `ci-integration.yml`, and `qa-smoke-preview-deploy.yml` set `env.BUN_VERSION` to that exact version; every first-party `oven-sh/setup-bun@v2` step uses `bun-version: ${{ env.BUN_VERSION }}`. - **Workflow pin verification:** `verify:bun-version` parses workflow YAML with Bun's built-in parser, so it also works before dependencies are installed. It checks each setup step's own `with.bun-version` and the workflow/job/step environment in scope; comments, run-script text, unrelated inputs, and another job's environment cannot satisfy the pin contract. Quoted scalars and YAML aliases remain supported. - **Live runtime verification:** `verify:vercel-build-controls` reads all three Vercel projects and requires `nodeVersion: "24.x"` with `bunVersion` absent or `null`. Any explicit Bun runtime value fails, even if source-controlled `vercel.json` files still select Node. This verifier only reads project settings. diff --git a/docs/guides/features/guest-giving-cover-fees.md b/docs/guides/features/guest-giving-cover-fees.md index ca1a8d4a9..464c931ba 100644 --- a/docs/guides/features/guest-giving-cover-fees.md +++ b/docs/guides/features/guest-giving-cover-fees.md @@ -47,8 +47,12 @@ Giving rows keep stored extras, including `payment_method`, because charged cents in `p_amount` do not preserve method. HTTP `POST /api/donate` replay of empty/legacy `{}` with matching charged -cents continues so the saga can persist the current extras onto empty. A -stored full quote that differs still returns `409`. +cents continues with the original empty extras and fee-related provider parameters. It must +not attach the current quote: the provider may have created a PaymentIntent +before the database completion write failed, and changing metadata or payment +methods under the same idempotency key would break that retry. A stored full +quote that differs still returns `409`; newly quoted gifts still persist their +quote at intake. ## Related diff --git a/docs/guides/operations/donation-saga-outbox.md b/docs/guides/operations/donation-saga-outbox.md index f3e8309cc..7953931f2 100644 --- a/docs/guides/operations/donation-saga-outbox.md +++ b/docs/guides/operations/donation-saga-outbox.md @@ -25,10 +25,10 @@ dollars. Gift processing-fee policy recomputes charged cents from `cover_fees` and `payment_method` before `begin_donation_saga`. `p_amount` is still charged cents. -First-shot processing from that POST persists quote extras onto -`donation_saga_outbox.fee_extras` and may attach them to PaymentIntent -metadata (`gift_amount_cents`, `cover_fees`, `payment_method`, -`cover_amount_cents`, `estimated_fee_cents`) without overriding `donation_id`. +Gift intake persists quote extras on the outbox in the same transaction that +creates the donation (`gift_amount_cents`, `cover_fees`, `payment_method`, +`cover_amount_cents`, `estimated_fee_cents`). Processing copies that quote to +PaymentIntent metadata without overriding donation identity. Recovery and batch workers (`processDueDonationSagaOutboxEvents`, admin replay) load stored `fee_extras` before PaymentIntent create. A lookup or @@ -53,11 +53,18 @@ stored `donations.amount` and `donation_saga_outbox.fee_extras` and: - returns `409` when charged cents match but a stored full fee quote differs from the current quote - continues when charged cents match and stored extras are empty/legacy `{}` - (or otherwise absent), passing the current quote extras so the saga can - persist onto empty before claim + (or otherwise absent), omitting fee metadata so the saga preserves its original + fee-related provider parameters and leaves empty stored extras unchanged - returns `500` when stored extras cannot be loaded or are malformed - processes the existing outbox without rewriting matching stored extras +An empty legacy quote is not evidence that Stripe has never seen the request. +The provider may have created a PaymentIntent before a database completion write +failed. Hydrating that retry with new fee metadata or payment-method types changes +its parameters under the same idempotency key and can strand the saga. This +replaces the earlier instruction to fill legacy extras; modern stored quotes and +first-shot intake persistence remain unchanged. + Verification: 1. POST the same idempotency key with a different charged amount → `409`. @@ -66,12 +73,44 @@ Verification: 3. POST the same key with matching charged cents and matching extras → `200` and no rewrite of stored extras. 4. POST the same key with matching charged cents and stored `fee_extras: {}` - → `200` and saga called with the current quote extras. + → `200` (or the existing processing response) with no fee metadata supplied + to the saga and no stored-extras rewrite. 5. POST `currency=eur` → `400` before `begin_donation_saga`. 6. First-shot card Gift PaymentIntents use `payment_method_types: ["card"]` and omit `automatic_payment_methods`. 7. Recovery of stored ACH extras binds `payment_method_types: ["us_bank_account"]` even when the worker omits extras. +8. Simulate provider success followed by a failed completion write, then POST + the same legacy gift again with the original actor and customer → the same + PaymentIntent completes, both provider requests have identical parameters, + and stored extras remain `{}`. Covered by + `packages/api/tests/unit/donate-post-charge.test.ts` using the real saga with + deterministic database and provider boundaries. + +### Replay migration rollout + +Apply `20261001053404_preserve_donation_saga_fee_replay.sql` before deploying +this HTTP replay path. The service-role-only fee-aware claim compares quotes +under a row lock before incrementing attempts. Conflicting requests return +`409` without recording a donation failure. The fee-extras trigger preserves +both full quotes and legacy absence after processing begins, including when an +older in-flight writer tries to hydrate the row. Existing worker claim and +recovery RPCs remain compatible. Keep the protective trigger during an +application rollback; pause donation processors before any database rollback. + +### Separate actor-recovery limitation + +This fee repair does not establish actor-independent provider recovery. The +background worker supplies `WORKFLOW_SYSTEM_ACTOR_ID`, while HTTP processing +supplies the authenticated actor; the saga currently copies that caller into +PaymentIntent `metadata.user_id`. A different retry actor can still change +provider parameters under the same idempotency key, even with an unchanged full +fee quote. The outbox does not retain the first provider caller. The intake audit +actor alone cannot reconstruct it because a worker may have sent the first +provider request. Do not guess historical attribution, remove it, or invent a +new payment identity to make recovery pass. Immutable first-provider identity +and an evidence-backed legacy recovery rule require a separate source-recovery +change; the regression above proves the same-actor HTTP fee replay only. Staff `POST /api/donations` does not run Gift processing-fee policy. That path already sends charged cents as `p_amount`. diff --git a/docs/ops/environments.md b/docs/ops/environments.md index 0be756452..8178e4ec0 100644 --- a/docs/ops/environments.md +++ b/docs/ops/environments.md @@ -113,11 +113,11 @@ the [Bun Functions runtime](https://vercel.com/docs/functions/runtimes/bun) (`"1.4.x"` or `"1.x"`), not the package manager. These projects stay on the Next.js + Node 24.x Functions runtime. -| Vercel project | `installCommand` | `buildCommand` | `ignoreCommand` | -| -------------- | ------------------------------------------- | -------------------------------------- | -------------------------------------------------------------- | -| `admin` | `bun install --cwd ../.. --frozen-lockfile` | `cd ../.. && bun run build:admin` | `node ../../scripts/vercel/should-ignore-build.mjs admin` | -| `donor` | `bun install --cwd ../.. --frozen-lockfile` | `cd ../.. && bun run build:donor` | `node ../../scripts/vercel/should-ignore-build.mjs donor` | -| `missionary` | `bun install --cwd ../.. --frozen-lockfile` | `cd ../.. && bun run build:missionary` | `node ../../scripts/vercel/should-ignore-build.mjs missionary` | +| Vercel project | `installCommand` | `buildCommand` | `ignoreCommand` | +| -------------- | ------------------------------------------------------ | -------------------------------------- | -------------------------------------------------------------- | +| `admin` | `bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile` | `cd ../.. && bun run build:admin` | `node ../../scripts/vercel/should-ignore-build.mjs admin` | +| `donor` | `bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile` | `cd ../.. && bun run build:donor` | `node ../../scripts/vercel/should-ignore-build.mjs donor` | +| `missionary` | `bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile` | `cd ../.. && bun run build:missionary` | `node ../../scripts/vercel/should-ignore-build.mjs missionary` | Vercel runs `ignoreCommand` from the app root. The helper returns `0` to skip the build and `1` to continue the build, matching Vercel's ignored-build diff --git a/docs/qa/pr-preview-smoke.md b/docs/qa/pr-preview-smoke.md index d4b5c6567..ddcdab7d2 100644 --- a/docs/qa/pr-preview-smoke.md +++ b/docs/qa/pr-preview-smoke.md @@ -72,6 +72,16 @@ Production. preview URL payload for Claude QA handoff. Missing webhook configuration is a skip, not a failure. +## Preview test identity + +Use a dedicated identity in the isolated preview datasource. The workflow +prefers `QA_PREVIEW_TEST_EMAIL` / `QA_PREVIEW_TEST_PASSWORD` and falls back to the +existing QA pair when the preview-specific pair is absent. The suite still +receives `QA_TEST_EMAIL` / `QA_TEST_PASSWORD`, so credential redaction is +unchanged. The identity must have legitimate access to the three tested +surfaces and the associated donor/missionary fixture records; login success +alone does not establish that access. + ## Required Secrets Configure these GitHub repository secrets: @@ -81,8 +91,9 @@ Configure these GitHub repository secrets: - `VERCEL_ADMIN_PROJECT_ID` - `VERCEL_DONOR_PROJECT_ID` - `VERCEL_MISSIONARY_PROJECT_ID` -- `QA_TEST_EMAIL` -- `QA_TEST_PASSWORD` +- `QA_PREVIEW_TEST_EMAIL` +- `QA_PREVIEW_TEST_PASSWORD` +- Legacy fallback: `QA_TEST_EMAIL` / `QA_TEST_PASSWORD` - `VERCEL_ADMIN_AUTOMATION_BYPASS_SECRET` - `VERCEL_DONOR_AUTOMATION_BYPASS_SECRET` - `VERCEL_MISSIONARY_AUTOMATION_BYPASS_SECRET` @@ -186,3 +197,5 @@ development and production deployments remain available. - [ ] No production deployment was requested - [ ] No secrets, credentials, tokens, cookies, reports, or bypass URLs were posted + +Configure both Preview credential secrets together. A partial Preview pair fails validation; the legacy QA pair is used only when both Preview secrets are absent. diff --git a/openspec/changes/guest-giving-gift-processing-fee-policy/design.md b/openspec/changes/guest-giving-gift-processing-fee-policy/design.md index a6d77680a..1b209cd10 100644 --- a/openspec/changes/guest-giving-gift-processing-fee-policy/design.md +++ b/openspec/changes/guest-giving-gift-processing-fee-policy/design.md @@ -70,9 +70,18 @@ Quote fields go on first-shot PaymentIntent metadata and on extras before PaymentIntent create. A lookup or parse failure must fail closed. An empty stored `{}` (GraphQL or legacy begin without a Gift quote) still omits `payment_method_types`. HTTP donate replay with matching charged cents treats -that empty/legacy default as absent, not as a colliding quote, so the saga can -persist the current extras onto empty before claim. A stored full quote that -differs from the current extras still `409`s. Recovery and batch first-shot +that empty/legacy default as absent, not as a colliding quote. It passes the +stored absence to the saga and leaves stored extras empty, preserving the +original fee metadata and payment-method parameters. Empty extras do not +prove that no provider request occurred: a PaymentIntent may have succeeded +before its database completion write failed. This replay-safety correction +supersedes the earlier instruction to fill legacy extras. A stored full quote +that differs from the current extras still `409`s. HTTP replay compares stored +extras under a database row lock before claiming, so a quote hydrated by an +older in-flight request is also checked without consuming a recovery attempt. +A database trigger freezes fee extras, including absence, once processing has +begun and prevents replacing an existing quote. Replays never persist caller extras. +Recovery and batch first-shot PaymentIntents MAY omit extras only for that empty/legacy `{}`; newly quoted Guest Giving rows keep stored extras including `payment_method` because `p_amount` does not preserve method. Documented in the donation-saga-outbox @@ -90,13 +99,45 @@ is Guest Giving Gift intake only. - ACH/wallet quotes can appear on the payment step while live confirm stays blocked. Tests lock the reject-before-POST behavior. - Estimated fee ≠ Stripe settlement. Copy must stay “estimated.” -- Persist-onto-empty HTTP donate replay is a first-write window, not CAS: - concurrent first quotes onto stored `{}` can race until one full quote - lands; later colliding full quotes `409`. Do not treat empty `{}` as an - immutable “no cover-fees” quote. +- Never hydrate an empty legacy quote during HTTP donate replay. The provider + may have seen the original request even if Core still needs to complete it. + Newly quoted gifts persist their quote atomically at intake; a later replay + cannot replace a stored full quote. Caller-actor metadata is a separate + pre-existing recovery limitation: the outbox does not retain the first + provider actor, so a worker retry can still change `metadata.user_id`. This + fee repair does not reconstruct that identity or prove actor-independent + recovery. See the operational guide for the bounded verification claim. ## Verification - Unit tests at the Core **interface**, schema defaults, Gift intake `p_amount`, saga metadata merge, checkout adapter POST body, and cover-fees UI. - `bun run openspec:validate`. + +### Preview validation prerequisite + +The exact-head admin preview failed before compilation because Vercel's build +image used Bun 1.3.14 against the Bun 1.4.0 frozen lockfile. Pin only installation +in the three app Vercel configs with `bunx bun@1.4.0 install`; keep Node Functions, +app build commands, deployment targeting, and the frozen lockfile unchanged. + +### Migration rollout + +Apply `20261001053404_preserve_donation_saga_fee_replay.sql` before deploying +HTTP replay code that calls the fee-aware claim RPC. The new RPC is invoker-only +and service-role-only; existing claim/recovery RPCs and RLS remain unchanged. +Older in-flight writers cannot change a quote after processing has started. +Application rollback can retain the protective trigger; database rollback +requires pausing donation processors first. The required migration CI job runs +the rollback-only SQL proof after the ordinary seed. + +### Hosted smoke identity + +Preview smoke exposed production-bound datasource/provider settings and a QA +identity that could not sign into the isolated test project. Preview-only +configuration now uses the dedicated development datasource and test-mode +provider keys. Use separate preview CI credentials for a test identity with +legitimate scoped surface access and donor/missionary fixture records. Restore +the committed membership lookup RPC in that test datasource; do not weaken +application authorization to satisfy smoke. Existing QA credentials remain the +workflow fallback. diff --git a/openspec/changes/guest-giving-gift-processing-fee-policy/proposal.md b/openspec/changes/guest-giving-gift-processing-fee-policy/proposal.md index cd56f21ae..10f92868c 100644 --- a/openspec/changes/guest-giving-gift-processing-fee-policy/proposal.md +++ b/openspec/changes/guest-giving-gift-processing-fee-policy/proposal.md @@ -17,8 +17,10 @@ saga creation. Checkout must stay a thin adapter; charged cents belong in Core. - Keep checkout as a thin adapter over that interface. Copy talks about estimated processing costs, never “100% reaches the field.” - Treat stored empty/legacy `{}` fee extras as absence, not as an immutable - “no cover-fees” quote. HTTP donate replay with matching charged cents may - persist the current extras onto that empty row; a stored full quote that + “no cover-fees” quote. HTTP donate replay with matching charged cents keeps + those extras empty and preserves the original fee-related provider parameters. A provider + request may already have succeeded before local completion failed; attaching + the current quote would break its idempotent retry. A stored full quote that differs still `409`s. - Do not rewrite allocation-line conservation of a payment group's gross amount. Do not apply cover-fees on the staff donations path. Do not enable diff --git a/openspec/changes/guest-giving-gift-processing-fee-policy/specs/donation-lifecycle/spec.md b/openspec/changes/guest-giving-gift-processing-fee-policy/specs/donation-lifecycle/spec.md index 51c1cfbd5..495bd2123 100644 --- a/openspec/changes/guest-giving-gift-processing-fee-policy/specs/donation-lifecycle/spec.md +++ b/openspec/changes/guest-giving-gift-processing-fee-policy/specs/donation-lifecycle/spec.md @@ -75,8 +75,18 @@ and does not reopen tenant processor-cost attribution. default - WHEN Gift intake evaluates the replay - THEN the request MUST continue instead of colliding -- AND the current extras MUST be passed so the saga can persist them onto - empty before claim +- AND intake MUST omit fee metadata for that legacy replay so the saga keeps + the original PaymentIntent parameters and does not fill empty stored extras +- AND a previously successful provider request whose completion write failed + MUST retain its provider idempotency key and unchanged fee metadata and + payment-method parameters; this fee rule does not establish caller-actor + identity recovery + +A legacy empty row does not prove that no provider request occurred. Adding a +current quote can change both metadata and payment-method selection after a +provider success, causing Stripe to reject the retry as an idempotency conflict. +This replaces the prior legacy-hydration clause; modern stored quotes remain +immutable and newly quoted gifts still persist their quote at intake. #### Scenario: Matching charged cents replay 409s when a stored full fee quote differs @@ -104,3 +114,18 @@ and does not reopen tenant processor-cost attribution. - WHEN the handler starts the donation saga - THEN `p_amount` remains the already-charged cents from the staff payload - AND Gift processing-fee policy MUST NOT run on that path + +#### Scenario: A rolling-deployment request hydrates legacy extras before replay claim + +- GIVEN HTTP intake reads empty legacy fee extras for a matching charged amount +- AND an older in-flight request stores a different full quote before saga claim +- WHEN the replay atomically validates the stored quote before claiming the outbox +- THEN it MUST return `409` before creating a customer or PaymentIntent +- AND it MUST NOT overwrite the stored fee extras or consume a recovery attempt + +#### Scenario: An older writer tries to hydrate a legacy row after processing begins + +- GIVEN a legacy donation has begun a provider attempt with absent fee extras +- WHEN an older in-flight request tries to write a new quote +- THEN the database MUST reject the quote change +- AND completion retry and worker recovery MUST retain the original fee-related provider parameters diff --git a/openspec/changes/guest-giving-gift-processing-fee-policy/tasks.md b/openspec/changes/guest-giving-gift-processing-fee-policy/tasks.md index 9e0cf812a..de23e6fc2 100644 --- a/openspec/changes/guest-giving-gift-processing-fee-policy/tasks.md +++ b/openspec/changes/guest-giving-gift-processing-fee-policy/tasks.md @@ -28,16 +28,34 @@ adapter POST body, and checkout cover-fees / ACH quote without live bank POST. - [x] 4.2 `bun run openspec -- validate guest-giving-gift-processing-fee-policy --type change --strict` passes. - `--all --strict` currently fails on unrelated pre-existing - `add-guest-giving-and-gift-anonymity` (receipts MODIFIED omits scenarios); - that change is out of scope. Archive this change after deployment - verification. + Full current-spec and active-change validation remains part of the + integration gate. Archive this change only after deployment verification. - [x] 4.3 Gift intake POST test asserts `begin_donation_saga` `p_amount` equals `resolveGiftIntakeCharge().chargedAmountCents`. - [x] 4.4 ADR-0118, runbook Guest Giving charged-amount section, and `docs/guides/features/guest-giving-cover-fees.md` document recovery extras and the staff-path exclusion. - [x] 4.5 HTTP donate replay unit tests lock matching charged cents + empty - or legacy `{}` continues (200) and persists current extras; matching - charged cents + a different stored full quote `409`s with no rewrite; - malformed stored extras `500`. + or legacy `{}` continues without rewriting extras or provider parameters; + matching charged cents + a different stored full quote `409`s with no + rewrite; malformed stored extras `500`. +- [x] 4.6 Reproduce provider success followed by a failed completion write through + the actual handler and saga; prove retry returns the same PaymentIntent + with identical provider parameters. Run focused tests, typechecking, and + strict OpenSpec validation after the replay-safety correction. + +- [x] 4.8 Cover matching-cents legacy card-cover and ACH-cover retries, an + unpersisted customer, modern quoted HTTP completion recovery, and a + late conflicting quote rejected before claim or provider calls. + +- [ ] 4.7 Complete `bun run ci:preflight` and applicable current-head CI and smoke + verification after the final integration-base update, then merge through + ordinary repository protections. + +- [ ] 4.9 Repair the verified preview install-tool mismatch by pinning Bun 1.4.0 + installation, verify the toolchain/build-control contracts, and rerun + current-head hosted smoke. + +- [x] 4.10 Validate the quote atomically before claim without consuming recovery + attempts; freeze fee extras once processing starts. Verify the actual + migration and rollback-only SQL proof on seeded disposable Postgres. diff --git a/packages/api/src/donate/index.ts b/packages/api/src/donate/index.ts index 6eb912eec..5804771a4 100644 --- a/packages/api/src/donate/index.ts +++ b/packages/api/src/donate/index.ts @@ -78,8 +78,9 @@ export const POST = withOperation( } const amountInCents = feeQuote.chargedAmountCents; const idempotencyKey = resolveRequiredIdempotencyKey(request.headers); - const extraPaymentIntentMetadata = - toGiftProcessingFeeStripeMetadata(feeQuote); + let extraPaymentIntentMetadata: + | GiftProcessingFeeStripeMetadata + | undefined = toGiftProcessingFeeStripeMetadata(feeQuote); const begin = await beginGiftIntake({ rpc: async (fn, rpcArgs) => { @@ -192,6 +193,9 @@ export const POST = withOperation( "This idempotency key was already used for a different gift fee quote.", ); } + // Legacy rows have no fee quote; retry with their original provider + // parameters instead of attaching the current request's quote. + extraPaymentIntentMetadata = storedFeeExtras; } const sagaResult = await processDonationSagaOutboxEvent({ @@ -200,6 +204,9 @@ export const POST = withOperation( outboxId: begin.outboxId, actorUserId: ctx.userId, extraPaymentIntentMetadata, + ...(begin.replayed + ? { replayFeeQuote: toGiftProcessingFeeStripeMetadata(feeQuote) } + : {}), }); if (sagaResult.status !== "completed") { diff --git a/packages/api/src/donate/saga.ts b/packages/api/src/donate/saga.ts index 941b4f640..2fe3239e0 100644 --- a/packages/api/src/donate/saga.ts +++ b/packages/api/src/donate/saga.ts @@ -11,6 +11,7 @@ import { mergeDonationPaymentIntentMetadata, type DonationPaymentIntentMethodType, } from "./payment-intent"; +import { ApiHttpError } from "../shared/api-http-error"; import type { getAdminClient } from "@asym/database/supabase/admin"; import type Stripe from "stripe"; @@ -25,6 +26,7 @@ interface DonationSagaProcessParams { outboxId: string; actorUserId: string; extraPaymentIntentMetadata?: GiftProcessingFeeStripeMetadata; + replayFeeQuote?: GiftProcessingFeeStripeMetadata; } interface DonationSagaProcessResult { @@ -38,6 +40,7 @@ interface DonationSagaProcessResult { interface DonationSagaClaimRow { claimed?: boolean; + fee_quote_conflict?: boolean; outbox_id?: string; donation_id?: string; donor_id?: string; @@ -281,6 +284,12 @@ async function processClaimedDonationSagaEvent(params: { throw new Error("Invalid donation amount in saga claim"); } + const extraPaymentIntentMetadata = await resolveDonationSagaFeeExtras({ + supabaseAdmin: params.supabaseAdmin, + outboxId: params.outboxId, + extraPaymentIntentMetadata: params.extraPaymentIntentMetadata, + }); + const stripeCustomerId = await ensureStripeCustomerId({ supabaseAdmin: params.supabaseAdmin, stripe: params.stripe, @@ -291,12 +300,6 @@ async function processClaimedDonationSagaEvent(params: { existingStripeCustomerId: stringOrNull(params.claim.stripe_customer_id), }); - const extraPaymentIntentMetadata = await resolveDonationSagaFeeExtras({ - supabaseAdmin: params.supabaseAdmin, - outboxId: params.outboxId, - extraPaymentIntentMetadata: params.extraPaymentIntentMetadata, - }); - const paymentIntent = await createDonationPaymentIntent(params.stripe, { amountCents: amount, currency, @@ -354,13 +357,14 @@ export async function processDonationSagaOutboxEvent({ outboxId, actorUserId, extraPaymentIntentMetadata, + replayFeeQuote, }: DonationSagaProcessParams): Promise { const lockId = randomUUID(); let lockClaimed = false; let lockedOutboxId = outboxId; try { - if (extraPaymentIntentMetadata) { + if (extraPaymentIntentMetadata && !replayFeeQuote) { await persistDonationSagaFeeExtras( supabaseAdmin, outboxId, @@ -369,10 +373,13 @@ export async function processDonationSagaOutboxEvent({ } const { data: claimRaw, error: claimError } = await supabaseAdmin.rpc( - "claim_donation_saga_event", + replayFeeQuote + ? "claim_donation_saga_event_with_fee_quote" + : "claim_donation_saga_event", { p_outbox_id: outboxId, p_lock_id: lockId, + ...(replayFeeQuote ? { p_expected_fee_extras: replayFeeQuote } : {}), }, ); @@ -381,6 +388,12 @@ export async function processDonationSagaOutboxEvent({ } const claim = parseRpcObject(claimRaw); + if (claim?.fee_quote_conflict) { + throw new ApiHttpError( + 409, + "This idempotency key was already used for a different gift fee quote.", + ); + } const claimed = Boolean(claim?.claimed); if (!claimed) { diff --git a/packages/api/tests/unit/donate-post-charge.test.ts b/packages/api/tests/unit/donate-post-charge.test.ts index 8c30327f4..dd74b012b 100644 --- a/packages/api/tests/unit/donate-post-charge.test.ts +++ b/packages/api/tests/unit/donate-post-charge.test.ts @@ -1,3 +1,5 @@ +import { isDeepStrictEqual } from "node:util"; + import { getAuthContext, type AuthContext } from "@asym/auth/context"; import { getAdminClient } from "@asym/database/supabase/admin"; import { createAuditLogger } from "@asym/lib/audit/logger"; @@ -341,6 +343,392 @@ describe("POST /api/donate Gift processing-fee policy", () => { expect(mockedProcessDonationSagaOutboxEvent).not.toHaveBeenCalled(); }); + it.each(["completed", "processing"] as const)( + "continues a matching legacy replay without new fee metadata when the saga is %s", + async (status) => { + mockedGetAdminClient.mockReturnValue({ + client: { + rpc: rpcMock, + from: createReplayFromMock({ + storedAmount: 10000, + storedFeeExtras: {}, + }), + } as never, + error: null, + }); + rpcMock.mockResolvedValue({ + data: { ...beginRpcResult, replayed: true }, + error: null, + }); + mockedProcessDonationSagaOutboxEvent.mockResolvedValue({ + status, + donationId: "donation-1", + outboxId: "outbox-1", + ...(status === "completed" + ? { paymentIntentId: "pi_legacy", clientSecret: "cs_legacy" } + : {}), + }); + + const response = await POST( + createDonateRequest({ amount: 100, currency: "usd" }), + ); + + expect(response.status).toBe(status === "completed" ? 200 : 202); + expect( + mockedProcessDonationSagaOutboxEvent, + ).toHaveBeenCalledExactlyOnceWith({ + supabaseAdmin: expect.anything(), + stripe: { id: "stripe-client" }, + outboxId: "outbox-1", + actorUserId: "user-1", + extraPaymentIntentMetadata: undefined, + replayFeeQuote: toGiftProcessingFeeStripeMetadata( + resolveGiftIntakeCharge({ + amount: 100, + coverFees: false, + paymentMethod: "card", + }), + ), + }); + expect(await response.json()).toMatchObject( + status === "completed" + ? { + paymentIntentId: "pi_legacy", + clientSecret: "cs_legacy", + replayed: true, + } + : { donationId: "donation-1", outboxId: "outbox-1", status }, + ); + }, + ); + + it.each([ + { + label: "legacy card", + amountCents: 10000, + paymentMethod: "card", + coverFees: false, + quoted: false, + missingCustomer: false, + changesDuringClaim: false, + }, + { + label: "legacy cover-card", + amountCents: 10330, + paymentMethod: "card", + coverFees: true, + quoted: false, + missingCustomer: false, + changesDuringClaim: false, + }, + { + label: "legacy cover-ACH with an unpersisted customer", + amountCents: 10081, + paymentMethod: "ach", + coverFees: true, + quoted: false, + missingCustomer: true, + changesDuringClaim: false, + }, + { + label: "new quoted cover-ACH with an unpersisted customer", + amountCents: 10081, + paymentMethod: "ach", + coverFees: true, + quoted: true, + missingCustomer: true, + changesDuringClaim: false, + }, + { + label: "conflicting quote before first claim", + amountCents: 10000, + paymentMethod: "card", + coverFees: false, + quoted: false, + missingCustomer: false, + changesDuringClaim: true, + }, + ] as const)( + "preserves payment replay state: $label", + async ({ + amountCents, + paymentMethod, + coverFees, + quoted, + missingCustomer, + changesDuringClaim, + }) => { + const requestBody = { + amount: 100, + currency: "usd", + cover_fees: coverFees, + payment_method: paymentMethod, + }; + const quote = toGiftProcessingFeeStripeMetadata( + resolveGiftIntakeCharge({ amount: 100, coverFees, paymentMethod }), + ); + const { processDonationSagaOutboxEvent: processActualSaga } = + await vi.importActual<{ + processDonationSagaOutboxEvent: typeof processDonationSagaOutboxEvent; + }>("../../src/donate/saga"); + const row: { + fee_extras: Record; + status: string; + attempts: number; + } = { fee_extras: quoted ? quote : {}, status: "pending", attempts: 0 }; + let completionFails = true; + const providerRequests = new Map(); + const createPaymentIntent = vi.fn( + async (params: unknown, options: { idempotencyKey: string }) => { + const previous = providerRequests.get(options.idempotencyKey); + if (previous && !isDeepStrictEqual(previous, params)) { + throw new Error("Idempotency key parameters changed on retry"); + } + providerRequests.set(options.idempotencyKey, params); + return { + id: "pi_legacy", + client_secret: "cs_legacy", + status: "requires_payment_method", + }; + }, + ); + const customerRequests = new Map(); + const createCustomer = vi.fn( + async (params: unknown, options: { idempotencyKey: string }) => { + const previous = customerRequests.get(options.idempotencyKey); + if (previous && !isDeepStrictEqual(previous, params)) { + throw new Error("Customer parameters changed on retry"); + } + customerRequests.set(options.idempotencyKey, params); + return { id: "cus_existing" }; + }, + ); + const stripe = { + paymentIntents: { create: createPaymentIntent }, + customers: { create: createCustomer }, + }; + const updateFeeExtras = vi.fn( + (value: { fee_extras: Record }) => ({ + eq: async () => { + row.fee_extras = value.fee_extras; + return { data: null, error: null }; + }, + }), + ); + const rpc = vi.fn(async (name: string, args: Record) => { + if (name === "begin_donation_saga") { + return { + data: { ...beginRpcResult, replayed: row.attempts > 0 || !quoted }, + error: null, + }; + } + if ( + name === "claim_donation_saga_event" || + name === "claim_donation_saga_event_with_fee_quote" + ) { + if (changesDuringClaim) { + row.fee_extras = toGiftProcessingFeeStripeMetadata( + resolveGiftIntakeCharge({ + amount: 100, + coverFees: false, + paymentMethod: "ach", + }), + ); + } + if ( + name === "claim_donation_saga_event_with_fee_quote" && + Object.keys(row.fee_extras).length > 0 && + !isDeepStrictEqual(row.fee_extras, args.p_expected_fee_extras) + ) { + return { + data: { claimed: false, fee_quote_conflict: true }, + error: null, + }; + } + row.status = "processing"; + row.attempts++; + return { + data: { + claimed: true, + donation_id: "donation-1", + donor_id: "donor-1", + tenant_id: "tenant-1", + amount: amountCents, + currency: "usd", + attempt_count: row.attempts, + idempotency_key: "guest-giving-fee-policy-test", + stripe_customer_id: missingCustomer ? null : "cus_existing", + }, + error: null, + }; + } + if (name === "complete_donation_saga_event") { + if (completionFails) { + completionFails = false; + return { + data: null, + error: { message: "completion write failed" }, + }; + } + row.status = "completed"; + return { data: { completed: true }, error: null }; + } + if (name === "record_donation_saga_failure") { + row.status = "pending"; + return { data: null, error: null }; + } + throw new Error(`Unexpected RPC: ${name}`); + }); + const readExtras = async () => ({ + data: { fee_extras: row.fee_extras }, + error: null, + }); + const from = vi.fn((table: string) => { + if (table === "donations") + return createDonationsFromMock(amountCents)(table); + if (table === "donors") + return { + select: () => ({ + eq: () => ({ + single: async () => ({ + data: { + id: "donor-1", + profile_id: "profile-1", + stripe_customer_id: null, + }, + error: null, + }), + }), + }), + }; + if (table === "profiles") + return { + select: () => ({ + eq: () => ({ + single: async () => ({ + data: { + email: "donor@example.com", + first_name: "Test", + last_name: "Donor", + }, + error: null, + }), + }), + }), + }; + expect(table).toBe("donation_saga_outbox"); + return { + select: () => ({ + eq: () => ({ + maybeSingle: readExtras, + eq: () => ({ single: readExtras }), + }), + }), + update: updateFeeExtras, + }; + }); + const client = { from, rpc }; + mockedGetAdminClient.mockReturnValue({ + client: client as never, + error: null, + }); + mockedResolveTenantStripe.mockResolvedValue({ + ok: true, + stripe: stripe as never, + secretKey: "rk_test_restricted", + publishableKey: "pk_test_123", + }); + mockedProcessDonationSagaOutboxEvent.mockImplementation( + processActualSaga, + ); + if (changesDuringClaim) { + expect(row.attempts).toBe(0); + } else if (quoted) { + const firstResponse = await POST(createDonateRequest(requestBody)); + expect(firstResponse.status).toBe(500); + } else { + await expect( + processActualSaga({ + supabaseAdmin: client as never, + stripe: stripe as never, + outboxId: "outbox-1", + actorUserId: "user-1", + }), + ).rejects.toThrow("completion write failed"); + } + expect(row.status).toBe("pending"); + expect(providerRequests.size).toBe(changesDuringClaim ? 0 : 1); + + const response = await POST(createDonateRequest(requestBody)); + + if (changesDuringClaim) { + expect(response.status).toBe(409); + expect(await response.json()).toMatchObject({ + error: + "This idempotency key was already used for a different gift fee quote.", + }); + expect(createPaymentIntent).not.toHaveBeenCalled(); + expect(createCustomer).not.toHaveBeenCalled(); + expect(updateFeeExtras).not.toHaveBeenCalled(); + expect(row.attempts).toBe(0); + expect(row.status).toBe("pending"); + expect( + rpc.mock.calls.some( + ([name]) => name === "record_donation_saga_failure", + ), + ).toBe(false); + return; + } + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + paymentIntentId: "pi_legacy", + clientSecret: "cs_legacy", + replayed: true, + }); + expect(row).toEqual({ + fee_extras: quoted ? quote : {}, + status: "completed", + attempts: 2, + }); + expect(updateFeeExtras).not.toHaveBeenCalled(); + expect(providerRequests.size).toBe(1); + expect(createPaymentIntent.mock.calls[0]?.[0]).toMatchObject( + quoted + ? { + amount: amountCents, + payment_method_types: ["us_bank_account"], + metadata: quote, + } + : { + amount: amountCents, + automatic_payment_methods: { enabled: true }, + }, + ); + if (!quoted) { + expect(createPaymentIntent.mock.calls[0]?.[0]).not.toHaveProperty( + "payment_method_types", + ); + expect(createPaymentIntent.mock.calls[0]?.[0]).not.toHaveProperty( + "metadata.gift_amount_cents", + ); + } + expect(createCustomer).toHaveBeenCalledTimes(missingCustomer ? 2 : 0); + if (missingCustomer) { + expect(customerRequests.size).toBe(1); + expect(createCustomer.mock.calls[1]).toEqual( + createCustomer.mock.calls[0], + ); + expect(createCustomer.mock.calls[0]?.[1]).toEqual({ + idempotencyKey: "guest-giving-fee-policy-test:customer", + }); + } + expect(createPaymentIntent).toHaveBeenCalledTimes(2); + expect(createPaymentIntent.mock.calls[1]).toEqual( + createPaymentIntent.mock.calls[0], + ); + }, + ); + it("replays a matching Gift with the current fee metadata so PI params stay bound", async () => { const expectedQuote = resolveGiftIntakeCharge({ amount: 100, @@ -415,8 +803,7 @@ describe("POST /api/donate Gift processing-fee policy", () => { expect(response.status).toBe(200); expect(mockedProcessDonationSagaOutboxEvent).toHaveBeenCalledWith( expect.objectContaining({ - extraPaymentIntentMetadata: - toGiftProcessingFeeStripeMetadata(expectedQuote), + extraPaymentIntentMetadata: undefined, }), ); }); diff --git a/scripts/verify/vercel-build-controls.mjs b/scripts/verify/vercel-build-controls.mjs index a578b2969..f5c37c9fa 100644 --- a/scripts/verify/vercel-build-controls.mjs +++ b/scripts/verify/vercel-build-controls.mjs @@ -20,7 +20,7 @@ export const EXPECTED_PROJECTS = Object.freeze([ projectId: "prj_SB9DucsrJOT0wF1v43SWMFsSNdn8", rootDirectory: "apps/admin", vercelConfigPath: "apps/admin/vercel.json", - installCommand: "bun install --cwd ../.. --frozen-lockfile", + installCommand: "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", buildCommand: "cd ../.. && bun run build:admin", ignoreCommand: "node ../../scripts/vercel/should-ignore-build.mjs admin", }), @@ -29,7 +29,7 @@ export const EXPECTED_PROJECTS = Object.freeze([ projectId: "prj_dZG3XkklLVZyqm85FW5Vvv7ph3kL", rootDirectory: "apps/donor", vercelConfigPath: "apps/donor/vercel.json", - installCommand: "bun install --cwd ../.. --frozen-lockfile", + installCommand: "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", buildCommand: "cd ../.. && bun run build:donor", ignoreCommand: "node ../../scripts/vercel/should-ignore-build.mjs donor", }), @@ -38,7 +38,7 @@ export const EXPECTED_PROJECTS = Object.freeze([ projectId: "prj_6tXSJKsdv2JpK70GKkg9HIg5hiYN", rootDirectory: "apps/missionary", vercelConfigPath: "apps/missionary/vercel.json", - installCommand: "bun install --cwd ../.. --frozen-lockfile", + installCommand: "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", buildCommand: "cd ../.. && bun run build:missionary", ignoreCommand: "node ../../scripts/vercel/should-ignore-build.mjs missionary", diff --git a/supabase/migrations/20261001053404_preserve_donation_saga_fee_replay.sql b/supabase/migrations/20261001053404_preserve_donation_saga_fee_replay.sql new file mode 100644 index 000000000..539c99f02 --- /dev/null +++ b/supabase/migrations/20261001053404_preserve_donation_saga_fee_replay.sql @@ -0,0 +1,98 @@ +-- Keep HTTP fee replay checks inside the claim transaction and freeze fee +-- parameters before any provider attempt, including the absence on legacy rows. +-- Existing claim and recovery RPC callers keep their original signatures. +-- Rollback (requires pausing donation processors first): +-- DROP TRIGGER IF EXISTS donation_saga_fee_extras_immutable ON public.donation_saga_outbox; +-- DROP FUNCTION IF EXISTS public.preserve_donation_saga_fee_extras(); +-- DROP FUNCTION IF EXISTS public.claim_donation_saga_event_with_fee_quote(UUID, UUID, JSONB); + +CREATE OR REPLACE FUNCTION public.preserve_donation_saga_fee_extras() +RETURNS TRIGGER +LANGUAGE plpgsql +SECURITY INVOKER +SET search_path = pg_catalog +AS $function$ +DECLARE + v_key TEXT; +BEGIN + IF NEW.fee_extras IS NOT DISTINCT FROM OLD.fee_extras THEN + RETURN NEW; + END IF; + + -- A stale ID-only writer must not hydrate a row after it has been claimed, + -- nor may a status/attempt reset in the same UPDATE evade the frozen state. + IF OLD.attempt_count > 0 OR NEW.attempt_count > 0 + OR OLD.status <> 'pending' OR NEW.status <> 'pending' THEN + RAISE EXCEPTION 'Donation saga fee extras are immutable after processing begins' + USING ERRCODE = '23514'; + END IF; + + -- Before claim, permit the initial legacy write but never replace an + -- existing quote. Match the TypeScript parser's five-field projection; + -- unrelated keys do not define fee identity. + IF OLD.fee_extras <> '{}'::jsonb THEN + FOREACH v_key IN ARRAY ARRAY[ + 'gift_amount_cents', 'cover_fees', 'payment_method', + 'cover_amount_cents', 'estimated_fee_cents' + ] LOOP + IF OLD.fee_extras->v_key IS DISTINCT FROM NEW.fee_extras->v_key THEN + RAISE EXCEPTION 'Donation saga fee quote cannot be replaced' + USING ERRCODE = '23514'; + END IF; + END LOOP; + END IF; + + RETURN NEW; +END; +$function$; + +REVOKE EXECUTE ON FUNCTION public.preserve_donation_saga_fee_extras() + FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION public.preserve_donation_saga_fee_extras() + TO service_role; + +CREATE TRIGGER donation_saga_fee_extras_immutable +BEFORE UPDATE OF fee_extras ON public.donation_saga_outbox +FOR EACH ROW EXECUTE FUNCTION public.preserve_donation_saga_fee_extras(); + +CREATE OR REPLACE FUNCTION public.claim_donation_saga_event_with_fee_quote( + p_outbox_id UUID, + p_lock_id UUID, + p_expected_fee_extras JSONB +) +RETURNS JSONB +LANGUAGE plpgsql +SECURITY INVOKER +SET search_path = pg_catalog +AS $function$ +DECLARE + v_row public.donation_saga_outbox%ROWTYPE; + v_key TEXT; +BEGIN + SELECT * INTO v_row + FROM public.donation_saga_outbox + WHERE id = p_outbox_id + FOR UPDATE; + + -- Compare before the existing claim increments attempts or acquires a + -- processor lock. Empty legacy extras stay absent and match any caller + -- quote; no fee metadata is ever persisted by this replay RPC. + IF FOUND AND v_row.fee_extras <> '{}'::jsonb THEN + FOREACH v_key IN ARRAY ARRAY[ + 'gift_amount_cents', 'cover_fees', 'payment_method', + 'cover_amount_cents', 'estimated_fee_cents' + ] LOOP + IF v_row.fee_extras->v_key IS DISTINCT FROM p_expected_fee_extras->v_key THEN + RETURN jsonb_build_object('claimed', FALSE, 'fee_quote_conflict', TRUE); + END IF; + END LOOP; + END IF; + + RETURN public.claim_donation_saga_event(p_outbox_id, p_lock_id); +END; +$function$; + +REVOKE EXECUTE ON FUNCTION public.claim_donation_saga_event_with_fee_quote(UUID, UUID, JSONB) + FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION public.claim_donation_saga_event_with_fee_quote(UUID, UUID, JSONB) + TO service_role; diff --git a/tests/integration/supabase/donation-fee-replay-verification.sql b/tests/integration/supabase/donation-fee-replay-verification.sql new file mode 100644 index 000000000..f09cbe2aa --- /dev/null +++ b/tests/integration/supabase/donation-fee-replay-verification.sql @@ -0,0 +1,147 @@ +-- Local, seeded-database proof for donation fee replay. All writes roll back. +\set ON_ERROR_STOP on + +BEGIN; +SET LOCAL request.jwt.claim.role = 'service_role'; + +DO $proof$ +DECLARE + v_tenant_id UUID; + v_profile_id UUID; + v_actor_id UUID; + v_intake JSONB; + v_outbox_id UUID; + v_lock_id UUID; + v_result JSONB; + v_before JSONB; + v_after JSONB; + v_case TEXT; + v_rejected BOOLEAN; + v_quote CONSTANT JSONB := '{"gift_amount_cents":"10000","cover_fees":"false","payment_method":"card","cover_amount_cents":"0","estimated_fee_cents":"320"}'; + v_other_quote CONSTANT JSONB := '{"gift_amount_cents":"10000","cover_fees":"false","payment_method":"ach","cover_amount_cents":"0","estimated_fee_cents":"80"}'; +BEGIN + IF has_function_privilege('anon', + 'public.claim_donation_saga_event_with_fee_quote(uuid,uuid,jsonb)', 'EXECUTE') + OR has_function_privilege('authenticated', + 'public.claim_donation_saga_event_with_fee_quote(uuid,uuid,jsonb)', 'EXECUTE') + OR NOT has_function_privilege('service_role', + 'public.claim_donation_saga_event_with_fee_quote(uuid,uuid,jsonb)', 'EXECUTE') THEN + RAISE EXCEPTION 'Fee-aware claim grants must be service-role-only'; + END IF; + IF EXISTS (SELECT 1 FROM pg_proc + WHERE oid = 'public.claim_donation_saga_event_with_fee_quote(uuid,uuid,jsonb)'::regprocedure + AND prosecdef) THEN + RAISE EXCEPTION 'Fee-aware claim must preserve invoker privileges'; + END IF; + + SELECT tenant_id, id, user_id + INTO v_tenant_id, v_profile_id, v_actor_id + FROM public.profiles WHERE tenant_id IS NOT NULL ORDER BY id LIMIT 1; + IF v_profile_id IS NULL THEN + RAISE EXCEPTION 'Run the repository seed before this proof'; + END IF; + + FOREACH v_case IN ARRAY ARRAY['quoted', 'legacy', 'hydrated'] LOOP + v_lock_id := gen_random_uuid(); + v_intake := public.begin_donation_saga( + p_tenant_id => v_tenant_id, p_profile_id => v_profile_id, + p_actor_user_id => v_actor_id, p_amount => 10000, + p_idempotency_key => 'fee-replay-proof-' || v_case, + p_fee_extras => CASE WHEN v_case = 'quoted' + THEN v_quote || '{"unrelated_key":"ignored by fee comparison"}'::jsonb + ELSE '{}'::jsonb END + ); + v_outbox_id := (v_intake->>'outbox_id')::uuid; + + IF v_case = 'hydrated' THEN + -- An older request may still do its first write before any claim. + UPDATE public.donation_saga_outbox SET fee_extras = v_quote + WHERE id = v_outbox_id; + END IF; + + IF v_case <> 'legacy' THEN + SELECT to_jsonb(o) INTO v_before FROM public.donation_saga_outbox o + WHERE id = v_outbox_id; + FOR i IN 1..6 LOOP + v_result := public.claim_donation_saga_event_with_fee_quote( + v_outbox_id, v_lock_id, v_other_quote); + IF v_result IS DISTINCT FROM '{"claimed":false,"fee_quote_conflict":true}'::jsonb THEN + RAISE EXCEPTION 'Different fee quote must reject before claim: %', v_result; + END IF; + END LOOP; + SELECT to_jsonb(o) INTO v_after FROM public.donation_saga_outbox o + WHERE id = v_outbox_id; + IF v_after IS DISTINCT FROM v_before THEN + RAISE EXCEPTION 'Conflicts must not consume attempts, change locks or record failures'; + END IF; + + v_rejected := FALSE; + BEGIN + UPDATE public.donation_saga_outbox SET fee_extras = v_other_quote + WHERE id = v_outbox_id; + EXCEPTION WHEN check_violation THEN v_rejected := TRUE; + END; + IF NOT v_rejected THEN RAISE EXCEPTION 'An existing quote must be immutable before claim'; END IF; + END IF; + + -- Equal quotes ignore unrelated keys. Empty legacy extras stay absent. + v_result := public.claim_donation_saga_event_with_fee_quote( + v_outbox_id, v_lock_id, v_quote); + IF (v_result->>'claimed')::boolean IS DISTINCT FROM TRUE + OR (v_result->>'attempt_count')::integer <> 1 THEN + RAISE EXCEPTION 'Matching quote or legacy absence must claim normally: %', v_result; + END IF; + + UPDATE public.donation_saga_outbox SET fee_extras = fee_extras WHERE id = v_outbox_id; + v_rejected := FALSE; + BEGIN + -- Simulates the stale ID-only write after the atomic replay claim. + UPDATE public.donation_saga_outbox SET fee_extras = v_other_quote + WHERE id = v_outbox_id; + EXCEPTION WHEN check_violation THEN v_rejected := TRUE; + END; + IF NOT v_rejected THEN RAISE EXCEPTION 'A claimed row must reject changed fee extras'; END IF; + + IF v_case = 'legacy' THEN + v_rejected := FALSE; + BEGIN + PERFORM public.complete_donation_saga_event( + v_outbox_id, gen_random_uuid(), 'pi_fee_replay_proof', NULL, '{}'::jsonb); + EXCEPTION WHEN no_data_found THEN v_rejected := TRUE; + END; + IF NOT v_rejected THEN RAISE EXCEPTION 'Fixture completion must fail for the wrong lock'; END IF; + PERFORM public.record_donation_saga_failure( + v_outbox_id, v_lock_id, 'completion_failed', 'proof completion failed', 60, 5, v_actor_id); + UPDATE public.donation_saga_outbox SET next_attempt_at = NOW() WHERE id = v_outbox_id; + v_rejected := FALSE; + BEGIN + UPDATE public.donation_saga_outbox SET fee_extras = v_quote WHERE id = v_outbox_id; + EXCEPTION WHEN check_violation THEN v_rejected := TRUE; + END; + IF NOT v_rejected THEN RAISE EXCEPTION 'Legacy absence must remain immutable after failed completion'; END IF; + v_lock_id := gen_random_uuid(); + -- Existing non-HTTP callers remain compatible with the original RPC. + v_result := public.claim_donation_saga_event(v_outbox_id, v_lock_id); + IF (v_result->>'claimed')::boolean IS DISTINCT FROM TRUE THEN + RAISE EXCEPTION 'Existing claim RPC must still recover legacy events'; + END IF; + END IF; + + PERFORM public.complete_donation_saga_event( + v_outbox_id, v_lock_id, 'pi_fee_replay_proof_' || v_case, NULL, '{}'::jsonb); + UPDATE public.donation_saga_outbox SET fee_extras = fee_extras WHERE id = v_outbox_id; + IF v_case = 'legacy' AND EXISTS ( + SELECT 1 FROM public.donation_saga_outbox + WHERE id = v_outbox_id AND fee_extras <> '{}'::jsonb + ) THEN RAISE EXCEPTION 'Legacy completion must preserve absent fee extras'; END IF; + v_rejected := FALSE; + BEGIN + UPDATE public.donation_saga_outbox SET fee_extras = v_other_quote WHERE id = v_outbox_id; + EXCEPTION WHEN check_violation THEN v_rejected := TRUE; + END; + IF NOT v_rejected THEN RAISE EXCEPTION 'Completed events must preserve provider fee parameters'; END IF; + END LOOP; +END; +$proof$; + +ROLLBACK; diff --git a/tests/unit/donation-saga.test.ts b/tests/unit/donation-saga.test.ts index cc6f1c3df..71bcbd435 100644 --- a/tests/unit/donation-saga.test.ts +++ b/tests/unit/donation-saga.test.ts @@ -34,7 +34,7 @@ function emptyOutboxFeeExtrasSelect() { } describe("donation saga helpers", () => { - it("processes a claimed event to completion", async () => { + it("processes a legacy event without rewriting its PaymentIntent parameters", async () => { const rpc = vi.fn().mockImplementation((fn: string) => { if (fn === "claim_donation_saga_event") { return Promise.resolve({ @@ -88,7 +88,23 @@ describe("donation saga helpers", () => { paymentIntentId: "pi_1", clientSecret: "secret_1", }); - expect(stripe.paymentIntents.create).toHaveBeenCalledTimes(1); + expect(stripe.paymentIntents.create).toHaveBeenCalledExactlyOnceWith( + { + amount: 5000, + currency: "usd", + customer: "cus_existing", + automatic_payment_methods: { enabled: true }, + metadata: { + donation_id: "don-1", + donor_id: "dor-1", + missionary_id: "", + fund_id: "", + tenant_id: "ten-1", + user_id: "usr-1", + }, + }, + { idempotencyKey: "idem-1:payment_intent" }, + ); expect(stripe.customers.create).not.toHaveBeenCalled(); expect(rpc).toHaveBeenCalledWith("complete_donation_saga_event", { p_outbox_id: "out-1", @@ -829,6 +845,80 @@ describe("donation saga helpers", () => { expect(stripe.paymentIntents.create).not.toHaveBeenCalled(); }); + it("rejects a resolved fee-extras write error before claiming or charging", async () => { + const extras = { + gift_amount_cents: "10000", + cover_fees: "true", + payment_method: "ach" as const, + cover_amount_cents: "81", + estimated_fee_cents: "81", + }; + const outbox = { fee_extras: {}, status: "pending" }; + const persistEq = vi.fn().mockResolvedValue({ + data: null, + error: { message: "fee extras write rejected" }, + }); + const update = vi.fn().mockReturnValue({ eq: persistEq }); + const maybeSingle = vi + .fn() + .mockResolvedValue({ data: outbox, error: null }); + const from = vi.fn(() => ({ + update, + select: vi.fn().mockReturnValue({ + eq: vi.fn().mockReturnValue({ maybeSingle }), + }), + })); + const rpc = vi.fn().mockImplementation((name: string) => { + if (name === "claim_donation_saga_event") { + outbox.status = "processing"; + return Promise.resolve({ + data: { + claimed: true, + donation_id: "don-write-error", + donor_id: "donor-write-error", + tenant_id: "tenant-write-error", + amount: 10081, + currency: "usd", + attempt_count: 1, + idempotency_key: "idem-write-error", + stripe_customer_id: "cus_write_error", + }, + error: null, + }); + } + if (name === "complete_donation_saga_event") { + outbox.status = "completed"; + return Promise.resolve({ data: { completed: true }, error: null }); + } + return Promise.resolve({ data: null, error: null }); + }); + const stripe = createStripeMock(); + ( + stripe.paymentIntents.create as ReturnType + ).mockResolvedValue({ + id: "pi_write_error", + client_secret: "cs_write_error", + status: "requires_payment_method", + }); + + await expect( + processDonationSagaOutboxEvent({ + supabaseAdmin: { rpc, from } as never, + stripe, + outboxId: "out-write-error", + actorUserId: "actor-write-error", + extraPaymentIntentMetadata: extras, + }), + ).rejects.toThrow("fee extras write rejected"); + + expect(update).toHaveBeenCalledWith({ fee_extras: extras }); + expect(persistEq).toHaveBeenCalledWith("id", "out-write-error"); + expect(rpc).not.toHaveBeenCalled(); + expect(stripe.customers.create).not.toHaveBeenCalled(); + expect(stripe.paymentIntents.create).not.toHaveBeenCalled(); + expect(outbox).toEqual({ fee_extras: {}, status: "pending" }); + }); + it("fails closed when Gift fee extras cannot be persisted before claim", async () => { const extras = { gift_amount_cents: "10000", diff --git a/tests/unit/scripts/bun-pin-sync.test.ts b/tests/unit/scripts/bun-pin-sync.test.ts index 667f1a140..1493078fa 100644 --- a/tests/unit/scripts/bun-pin-sync.test.ts +++ b/tests/unit/scripts/bun-pin-sync.test.ts @@ -191,7 +191,7 @@ describe("Bun toolchain pin sync", () => { `${app} vercel.json bunVersion (Functions runtime opt-in)`, ).toBeUndefined(); expect(vercelConfig.installCommand, `${app} installCommand`).toBe( - "bun install --cwd ../.. --frozen-lockfile", + `bunx bun@${VERIFIED_STABLE_BUN} install --cwd ../.. --frozen-lockfile`, ); expect(vercelConfig.installCommand).not.toContain("--save-text-lockfile"); expect(buildCommand, `${app} buildCommand`).not.toMatch(/--bun\b/); diff --git a/tests/unit/scripts/ci-integration-workflow.contract.test.ts b/tests/unit/scripts/ci-integration-workflow.contract.test.ts index 85378090b..860df3a9f 100644 --- a/tests/unit/scripts/ci-integration-workflow.contract.test.ts +++ b/tests/unit/scripts/ci-integration-workflow.contract.test.ts @@ -61,6 +61,16 @@ describe("ci-integration workflow contract", () => { const workflow = readWorkflow(); const scripts = readPackageScripts(); + it("runs the donation fee replay SQL proof after seeding in the required migration job", () => { + const migrate = jobBlock(workflow, "migrate"); + expect(migrate).toContain( + "tests/integration/supabase/donation-fee-replay-verification.sql", + ); + expect(migrate.indexOf("Apply demo seed")).toBeLessThan( + migrate.indexOf("Verify donation fee replay"), + ); + }); + it("keeps develop merges gated by smoke while full E2E stays informational", () => { const testE2e = jobBlock(workflow, "test-e2e"); const testE2eSmoke = jobBlock(workflow, "test-e2e-smoke"); diff --git a/tests/unit/scripts/vercel-build-controls.test.ts b/tests/unit/scripts/vercel-build-controls.test.ts index eba4df2eb..c45d4f665 100644 --- a/tests/unit/scripts/vercel-build-controls.test.ts +++ b/tests/unit/scripts/vercel-build-controls.test.ts @@ -15,7 +15,7 @@ const adminProject = EXPECTED_PROJECTS[0]!; const localConfig = { $schema: "https://openapi.vercel.sh/vercel.json", - installCommand: "bun install --cwd ../.. --frozen-lockfile", + installCommand: "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", buildCommand: "cd ../.. && bun run build:admin", ignoreCommand: "node ../../scripts/vercel/should-ignore-build.mjs admin", git: { @@ -47,7 +47,7 @@ describe("Vercel build controls verifier", () => { projectId: "prj_SB9DucsrJOT0wF1v43SWMFsSNdn8", rootDirectory: "apps/admin", vercelConfigPath: "apps/admin/vercel.json", - installCommand: "bun install --cwd ../.. --frozen-lockfile", + installCommand: "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", buildCommand: "cd ../.. && bun run build:admin", ignoreCommand: "node ../../scripts/vercel/should-ignore-build.mjs admin", @@ -57,7 +57,7 @@ describe("Vercel build controls verifier", () => { projectId: "prj_dZG3XkklLVZyqm85FW5Vvv7ph3kL", rootDirectory: "apps/donor", vercelConfigPath: "apps/donor/vercel.json", - installCommand: "bun install --cwd ../.. --frozen-lockfile", + installCommand: "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", buildCommand: "cd ../.. && bun run build:donor", ignoreCommand: "node ../../scripts/vercel/should-ignore-build.mjs donor", @@ -67,7 +67,7 @@ describe("Vercel build controls verifier", () => { projectId: "prj_6tXSJKsdv2JpK70GKkg9HIg5hiYN", rootDirectory: "apps/missionary", vercelConfigPath: "apps/missionary/vercel.json", - installCommand: "bun install --cwd ../.. --frozen-lockfile", + installCommand: "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", buildCommand: "cd ../.. && bun run build:missionary", ignoreCommand: "node ../../scripts/vercel/should-ignore-build.mjs missionary", @@ -228,7 +228,7 @@ describe("Vercel build controls verifier", () => { nodeVersion: "24.x", bunVersion: null, buildCommand: "bun run build", - installCommand: "bun install --cwd ../.. --frozen-lockfile", + installCommand: "bunx bun@1.4.0 install --cwd ../.. --frozen-lockfile", enableAffectedProjectsDeployments: true, previewDeploymentsDisabled: true, resourceConfig: { diff --git a/tests/unit/workflows/qa-smoke-preview-deploy.test.ts b/tests/unit/workflows/qa-smoke-preview-deploy.test.ts index 2e3f7a6ea..25acb7576 100644 --- a/tests/unit/workflows/qa-smoke-preview-deploy.test.ts +++ b/tests/unit/workflows/qa-smoke-preview-deploy.test.ts @@ -57,6 +57,24 @@ describe("qa smoke preview deployment workflow", () => { } }); + it("prefers isolated preview credentials while keeping existing QA credentials as fallback", () => { + expect( + workflow.match( + /QA_TEST_EMAIL: \$\{\{ secrets\.QA_PREVIEW_TEST_EMAIL && secrets\.QA_PREVIEW_TEST_PASSWORD && secrets\.QA_PREVIEW_TEST_EMAIL \|\| secrets\.QA_TEST_EMAIL \}\}/g, + ), + ).toHaveLength(3); + expect( + workflow.match( + /QA_TEST_PASSWORD: \$\{\{ secrets\.QA_PREVIEW_TEST_EMAIL && secrets\.QA_PREVIEW_TEST_PASSWORD && secrets\.QA_PREVIEW_TEST_PASSWORD \|\| secrets\.QA_TEST_PASSWORD \}\}/g, + ), + ).toHaveLength(3); + expect(workflow).toContain("QA_PREVIEW_CREDENTIALS_PARTIAL:"); + expect(workflow).toContain('"${QA_PREVIEW_CREDENTIALS_PARTIAL}" == "true"'); + expect(workflow).toContain( + "Both QA_PREVIEW_TEST_EMAIL and QA_PREVIEW_TEST_PASSWORD must be configured together.", + ); + }); + it("uses the required Vercel deployment and Playwright smoke secrets", () => { expect(workflow).toContain("secrets.VERCEL_ADMIN_PROJECT_ID"); expect(workflow).toContain("secrets.VERCEL_DONOR_PROJECT_ID");