fix: sync org names to Stripe#1910
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Bun/TypeScript bulk sync script to resynchronize Stripe customer Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Handler as PUT /organization Handler
participant DB as Supabase Database
participant Stripe as Stripe API
Client->>Handler: PUT /organization (name)
Handler->>DB: SELECT current org row (id, name, customer_id)
DB-->>Handler: existing org
Handler->>Handler: sanitize name
alt customer_id exists & not pending_ & name changed
Handler->>Stripe: customers.update(customer_id, { name: newName })
Stripe-->>Handler: success/failure
end
Handler->>DB: UPDATE org SET name = newName (with .eq('name', oldName) / maybeSingle)
alt DB update succeeds
DB-->>Handler: OK
Handler-->>Client: 200 + updated org
else DB update fails
DB-->>Handler: Error
alt Stripe sync was attempted
Handler->>Stripe: customers.update(customer_id, { name: oldName }) // rollback
Stripe-->>Handler: rollback response
end
Handler-->>Client: Error (includes rollback status)
end
sequenceDiagram
actor Admin
participant Script as sync_stripe_org_names.ts
participant Env as Env Loader
participant DB as Supabase Database
participant Stripe as Stripe API
participant FS as Local FS
Admin->>Script: Run (args: --apply/--org-id/--limit/--concurrency)
Script->>Env: load env file + Bun.env
Script->>DB: Fetch target orgs (by org_id OR paginated where customer_id NOT NULL)
DB-->>Script: list of {id, name, customer_id}
Script->>Script: filter out empty names and pending_ customer_ids
Script->>Script: split into batches & async pool (concurrency)
par Parallel updates
Script->>Stripe: customers.update(customer_id, { name: org.name })
Stripe-->>Script: success/failure
end
Script->>Script: aggregate results
alt failures found
Script->>FS: write ./tmp/stripe_org_name_sync_failures.json
Script-->>Admin: throw error / report failures
else all succeed
Script-->>Admin: report success count & samples
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing 
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22424ff15e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/stripe-redirects.unit.test.ts (1)
379-420: Consider usingit.concurrent()for parallel test execution.These tests are independent and could run concurrently. As per coding guidelines, use
it.concurrent()instead ofit()when possible to maximize parallelism for faster CI/CD.♻️ Suggested refactor
- it('updates Stripe customer email without overwriting the organization name', async () => { + it.concurrent('updates Stripe customer email without overwriting the organization name', async () => {- it('updates Stripe customer name when the organization name changes', async () => { + it.concurrent('updates Stripe customer name when the organization name changes', async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/stripe-redirects.unit.test.ts` around lines 379 - 420, Replace the two independent tests that use it(...) with concurrent tests by changing it('updates Stripe customer email without overwriting the organization name', ...) to it.concurrent('updates Stripe customer email without overwriting the organization name', ...) and it('updates Stripe customer name when the organization name changes', ...) to it.concurrent('updates Stripe customer name when the organization name changes', ...); keep the existing setup and assertions (mocks for Stripe, updateCustomer, and imports of updateCustomerEmail and updateCustomerOrganizationName) unchanged so tests run safely in parallel.tests/organization-put-stripe-sync.unit.test.ts (1)
81-133: Consider usingit.concurrent()for parallel test execution.These tests reset mocks in
beforeEachand are independent. Per coding guidelines, useit.concurrent()when possible to maximize CI/CD parallelism.♻️ Suggested refactor
- it('syncs Stripe customer name before updating the org row', async () => { + it.concurrent('syncs Stripe customer name before updating the org row', async () => {- it('rolls Stripe back when the database update fails', async () => { + it.concurrent('rolls Stripe back when the database update fails', async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/organization-put-stripe-sync.unit.test.ts` around lines 81 - 133, Replace the two independent tests that currently use it(...) with concurrent execution by calling it.concurrent(...) for the specs titled "syncs Stripe customer name before updating the org row" and "rolls Stripe back when the database update fails"; update the two invocations that set up mocks via supabaseClientMock, createOrgSelectBuilder and createOrgUpdateBuilder and the assertions around put(...) and updateCustomerOrganizationNameMock to remain unchanged so the tests run in parallel safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/sync_stripe_org_names.ts`:
- Around line 182-184: The filter chain that builds actionableOrgs calls
org.name.trim() unguarded and will throw if org.name is null; update the filter
on org.name to use a null-safe check such as optional chaining and a falsy guard
(e.g. org.name?.trim() and/or typeof check) so only non-empty string names pass.
Modify the second filter in actionableOrgs (the .filter(org =>
!!org.name.trim()) predicate) to safely handle null/undefined names before
calling trim.
- Around line 232-234: The write to FAILURE_OUTPUT may fail if its parent ./tmp/
directory doesn't exist; before calling Bun.write in the failures block (the
conditional that checks failures.length and calls Bun.write), ensure the
directory for FAILURE_OUTPUT exists by creating its parent directory (use
Bun.mkdir or equivalent with recursive:true) so the file write succeeds; update
the failures handling block around FAILURE_OUTPUT and Bun.write to create the
directory first, then perform the write and log.
---
Nitpick comments:
In `@tests/organization-put-stripe-sync.unit.test.ts`:
- Around line 81-133: Replace the two independent tests that currently use
it(...) with concurrent execution by calling it.concurrent(...) for the specs
titled "syncs Stripe customer name before updating the org row" and "rolls
Stripe back when the database update fails"; update the two invocations that set
up mocks via supabaseClientMock, createOrgSelectBuilder and
createOrgUpdateBuilder and the assertions around put(...) and
updateCustomerOrganizationNameMock to remain unchanged so the tests run in
parallel safely.
In `@tests/stripe-redirects.unit.test.ts`:
- Around line 379-420: Replace the two independent tests that use it(...) with
concurrent tests by changing it('updates Stripe customer email without
overwriting the organization name', ...) to it.concurrent('updates Stripe
customer email without overwriting the organization name', ...) and it('updates
Stripe customer name when the organization name changes', ...) to
it.concurrent('updates Stripe customer name when the organization name changes',
...); keep the existing setup and assertions (mocks for Stripe, updateCustomer,
and imports of updateCustomerEmail and updateCustomerOrganizationName) unchanged
so tests run safely in parallel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00933558-a7bb-49ac-8d91-a385f358f8ff
📒 Files selected for processing (7)
package.jsonscripts/sync_stripe_org_names.tssupabase/functions/_backend/public/organization/put.tssupabase/functions/_backend/utils/stripe.tstests/organization-api.test.tstests/organization-put-stripe-sync.unit.test.tstests/stripe-redirects.unit.test.ts
💤 Files with no reviewable changes (1)
- tests/organization-api.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/organization-put-stripe-sync.unit.test.ts (1)
111-133: Consider adding test coverage for failed rollback scenario.This test validates the rollback scenario and correctly asserts that
updateCustomerOrganizationNameis called twice: first with the new name and then with the old name. However, the test mocks both calls to succeed, so it doesn't test what happens if either the forward update or the rollback update throws an error.💡 Suggested additional test
it('includes both errors when rollback also fails', async () => { const selectBuilder = createOrgSelectBuilder({ id: 'org-123', name: 'Old Name', customer_id: 'cus_123', }) const updateBuilder = createOrgUpdateBuilder(null, { message: 'db write failed' }) supabaseClientMock.mockReturnValue({ from: vi.fn() .mockReturnValueOnce(selectBuilder) .mockReturnValueOnce(updateBuilder), }) // First call succeeds, rollback call fails updateCustomerOrganizationNameMock .mockResolvedValueOnce(undefined) .mockRejectedValueOnce(new Error('Stripe rollback failed')) const error = await put(createContext(), { orgId: 'org-123', name: 'New Name', }, undefined).catch(caught => caught) expect(error).toBeInstanceOf(HTTPException) // Verify error contains both original and rollback error info })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/organization-put-stripe-sync.unit.test.ts` around lines 111 - 133, Add a new unit test that simulates a failing DB update and a failing Stripe rollback by reusing createOrgSelectBuilder and createOrgUpdateBuilder, mocking supabaseClientMock to return those builders, then configuring updateCustomerOrganizationNameMock to resolve once (forward update succeeds) and reject once (rollback fails). Call the put(context, { orgId, name }, undefined) and capture the thrown HTTPException, asserting it's an HTTPException and that its message or nested error payload includes both the original DB error ("db write failed") and the rollback error ("Stripe rollback failed") to ensure both errors are surfaced by put.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/organization-put-stripe-sync.unit.test.ts`:
- Around line 111-133: Add a new unit test that simulates a failing DB update
and a failing Stripe rollback by reusing createOrgSelectBuilder and
createOrgUpdateBuilder, mocking supabaseClientMock to return those builders,
then configuring updateCustomerOrganizationNameMock to resolve once (forward
update succeeds) and reject once (rollback fails). Call the put(context, {
orgId, name }, undefined) and capture the thrown HTTPException, asserting it's
an HTTPException and that its message or nested error payload includes both the
original DB error ("db write failed") and the rollback error ("Stripe rollback
failed") to ensure both errors are surfaced by put.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09784bab-a6e4-478b-9c0f-c26ae151cd39
📒 Files selected for processing (3)
scripts/sync_stripe_org_names.tssupabase/functions/_backend/public/organization/put.tstests/organization-put-stripe-sync.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/sync_stripe_org_names.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c62e001bef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f08f0d73cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…ariable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a888a8032b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f30947a7b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
supabase/functions/_backend/public/organization/put.ts (1)
213-247: Add request-scoped logs around the Stripe sync and rollback branches.This path now contains multiple external writes and failure modes, but there is still no structured log tying them to a
requestId. When rollback lookup/update fails, reconciling DB vs Stripe drift will be much harder than it needs to be.As per coding guidelines, "All endpoints must receive Hono
Context<MiddlewareKeyVariables>object and usec.get('requestId')for structured logging withcloudlog()".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/public/organization/put.ts` around lines 213 - 247, Add structured, request-scoped logging (using c.get('requestId') and cloudlog()) around the Stripe sync and rollback branches so every external write and failure includes the requestId and context: log before calling updateCustomerOrganizationName(c, ...), after success, and on error; log the beginning and result of updateOrg(...) and include the expectedCurrentName context; when catching errors and performing rollback, log the getOrgForNameSync(...) lookup result/failure and the subsequent updateCustomerOrganizationName(...) rollback attempt and failure, referencing currentOrg, rollbackOrg, shouldUpdateStripeCustomerName and the original error details so the cloud logs show requestId, operation, inputs and error messages for easier reconciliation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/public/organization/put.ts`:
- Around line 89-102: sanitizeOrgNameForSync currently returns whatever
strip_html RPC yields, but that can be empty/whitespace and later gets written
to orgs.name and Stripe; after calling supabase.rpc('strip_html') in
sanitizeOrgNameForSync validate the returned data by trimming it and rejecting
if it's empty (throw the existing simpleError with a clear error payload, e.g.,
error: 'sanitized_name_empty' or similar), otherwise return the trimmed
non-empty string so downstream code never receives a blank organization name.
- Around line 213-247: The current flow updates Stripe via
updateCustomerOrganizationName before committing the DB (updateOrg) and attempts
a best-effort rollback using getOrgForNameSync on error, which can race with
concurrent renames; change to drive Stripe updates from committed DB state
instead: remove the pre-commit call to updateCustomerOrganizationName(c, ...)
and instead perform the Stripe rename after updateOrg succeeds or, better, write
an outbox/event row (e.g., org name change record) inside the same transaction
used by updateOrg and have a separate reconciler/worker read that committed
outbox and call updateCustomerOrganizationName; ensure the reconciler uses the
DB’s canonical name and idempotent calls (and leverages expectedCurrentName/
customer_id checks) so concurrent renames cannot cause Stripe to be rolled back
to a stale name.
In `@tests/cli-min-version.test.ts`:
- Around line 109-120: The test is flaky because transient network failures
aren't retried for sdk calls and uploads; wrap the SDK calls (sdk.addChannel and
sdk.updateChannel that produce createChannelResult and linkSeedBundleResult) in
the same retry logic used elsewhere (or call the existing retry helper) so they
retry on transient "fetch failed" or network errors, and replace direct upload
calls referenced at lines like 163-167 and 201-208 with retryUpload (or the
shared retry wrapper) so assertions only run after a successful retried
operation.
---
Nitpick comments:
In `@supabase/functions/_backend/public/organization/put.ts`:
- Around line 213-247: Add structured, request-scoped logging (using
c.get('requestId') and cloudlog()) around the Stripe sync and rollback branches
so every external write and failure includes the requestId and context: log
before calling updateCustomerOrganizationName(c, ...), after success, and on
error; log the beginning and result of updateOrg(...) and include the
expectedCurrentName context; when catching errors and performing rollback, log
the getOrgForNameSync(...) lookup result/failure and the subsequent
updateCustomerOrganizationName(...) rollback attempt and failure, referencing
currentOrg, rollbackOrg, shouldUpdateStripeCustomerName and the original error
details so the cloud logs show requestId, operation, inputs and error messages
for easier reconciliation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80c43ab3-8d1e-43ba-a31c-dbb36845f9e9
📒 Files selected for processing (6)
package.jsonsupabase/functions/_backend/public/organization/put.tstests/cli-min-version.test.tstests/device.test.tstests/events.test.tstests/organization-put-stripe-sync.unit.test.ts
✅ Files skipped from review due to trivial changes (2)
- package.json
- tests/organization-put-stripe-sync.unit.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b25aee9694
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7abb8dc4f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74ace57c44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd2155d047
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c11056a565
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
2 participants
Summary (AI generated)
Motivation (AI generated)
Organization renames updated Capgo data but left Stripe customer names stale. That drift made billing records inconsistent and required manual cleanup for existing organizations.
Business Impact (AI generated)
This keeps billing data aligned with customer-facing organization records, reduces manual support work, and gives the team a safe bulk resync path for historical mismatches.
Test Plan (AI generated)
bunx vitest run tests/organization-put-stripe-sync.unit.test.ts tests/stripe-redirects.unit.test.tsbunx eslint supabase/functions/_backend/public/organization/put.ts supabase/functions/_backend/utils/stripe.ts tests/organization-put-stripe-sync.unit.test.ts tests/stripe-redirects.unit.test.ts tests/organization-api.test.ts --no-warn-ignoredbunx eslint scripts/sync_stripe_org_names.ts --no-ignorebun run supabase:with-env -- bunx vitest run tests/organization-api.test.ts -t "update organization"(blocked locally: Docker daemon/OrbStack not running, so Supabase could not start)Generated with AI
Summary by CodeRabbit
New Features
Chores
Bug Fix
Tests