diff --git a/modules/billing/billing.init.js b/modules/billing/billing.init.js index 070924aab..dbca573d0 100644 --- a/modules/billing/billing.init.js +++ b/modules/billing/billing.init.js @@ -9,6 +9,7 @@ import billingEvents from './lib/events.js'; import invitationEvents from '../invitations/lib/events.js'; import organizationEvents from '../organizations/lib/events.js'; import BillingUsageRepository from './repositories/billing.usage.repository.js'; +import BillingSignupGrantService from './services/billing.signupGrant.service.js'; import { getAlertThresholdPercents } from './lib/billing.constants.js'; import { setupBillingEmails } from './billing.email.js'; @@ -132,6 +133,40 @@ export default async (app) => { } }); + // Signup grant (#3952) — billing is an OPTIONAL consumer of the organizations fire-and-forget + // `organization.created` event (dependency direction billing → organizations, same as + // `organization.provisioned` above: billing imports the events singleton, organizations never + // imports billing). BOTH organization-creation call sites (organizations.crud.service.js::create + // AND organizations.service.js::createOrganizationForUser) emit this instead of calling + // BillingSignupGrantService directly, so organizations stays removable without billing. + // Unlike the two referral listeners above, this one is NOT config-gated — grantOnSignup is core + // signup behavior (it already no-ops when the plan defines no signupGrant). + /** + * @desc One-shot signup grant listener for organization-creation events (#3952). + * Credits the configured signupGrant to the freshly created organization via + * BillingSignupGrantService.grantOnSignup (idempotent via refId `signup_grant-`, + * never throws). Self-guarded: `EventEmitter.emit` is synchronous, so the emit-site + * try/catch in organizations only catches SYNC throws — an async rejection escaping + * here would surface as an unhandledRejection. It never does: grantOnSignup already + * swallows its own errors; this wraps it too for defense-in-depth (mirrors the instant + * referee grant listener above). + * @param {{orgId: string, planId: string}} payload - Created organization event payload. + * @returns {Promise} settles when the grant attempt completes (never rejects) + */ + organizationEvents.on('organization.created', async (payload) => { + try { + await BillingSignupGrantService.grantOnSignup({ orgId: payload?.orgId, planId: payload?.planId }); + } catch (err) { + // ⚠️ MANDATORY self-guard (see organizations/lib/events.js): never let a rejection escape. + logger.error('[billing] signup grant failed via organization.created listener', { + orgId: String(payload?.orgId ?? ''), + planId: String(payload?.planId ?? ''), + err: err?.message, + stack: err?.stack, + }); + } + }); + // Update analytics group properties when a subscription plan changes billingEvents.on('plan.changed', ({ organizationId, newPlan }) => { try { diff --git a/modules/billing/tests/billing.init.unit.tests.js b/modules/billing/tests/billing.init.unit.tests.js index 4ab40397f..adae32ebb 100644 --- a/modules/billing/tests/billing.init.unit.tests.js +++ b/modules/billing/tests/billing.init.unit.tests.js @@ -18,6 +18,7 @@ describe('billing.init unit tests:', () => { let mockOrganizationEvents; let mockUserService; let mockInvitationRepository; + let mockSignupGrantService; const mockApp = {}; @@ -94,6 +95,13 @@ describe('billing.init unit tests:', () => { default: mockInvitationRepository, })); + // #3952: the organization.created listener delegates to BillingSignupGrantService — stub it + // so the wiring tests stay isolated from the real grant/plan/repository resolution. + mockSignupGrantService = { grantOnSignup: jest.fn().mockResolvedValue({ applied: true }) }; + jest.unstable_mockModule('../services/billing.signupGrant.service.js', () => ({ + default: mockSignupGrantService, + })); + // Stub billing.email so boot validator tests don't wire real email listeners jest.unstable_mockModule('../billing.email.js', () => ({ setupBillingEmails: jest.fn(), @@ -273,6 +281,57 @@ describe('billing.init unit tests:', () => { }); }); + describe('#3952 signup grant listener (organization.created):', () => { + const payload = { orgId: 'org1', planId: 'free' }; + + /** + * Boot the module and return the registered organization.created handler. + * @returns {Promise} The wired listener. + */ + const getHandler = async () => { + await billingInit(mockApp); + const createdCall = mockOrganizationEvents.on.mock.calls.find(([evt]) => evt === 'organization.created'); + expect(createdCall).toBeDefined(); + return createdCall[1]; + }; + + test('wires the listener on the organizations emitter', async () => { + const handler = await getHandler(); + expect(typeof handler).toBe('function'); + }); + + test('NOT config-gated — delegates to BillingSignupGrantService.grantOnSignup with no referral config set', async () => { + // mockConfig.billing has NO referral block — unlike the two referral listeners above, + // this one must still fire (grantOnSignup is core signup behavior, not a referral reward). + const handler = await getHandler(); + await handler(payload); + expect(mockSignupGrantService.grantOnSignup).toHaveBeenCalledTimes(1); + expect(mockSignupGrantService.grantOnSignup).toHaveBeenCalledWith({ orgId: 'org1', planId: 'free' }); + }); + + test('passes the payload through verbatim (orgId + planId from both org-creation call sites)', async () => { + const handler = await getHandler(); + await handler({ orgId: 'org2', planId: 'growth' }); + expect(mockSignupGrantService.grantOnSignup).toHaveBeenCalledWith({ orgId: 'org2', planId: 'growth' }); + }); + + test('self-guard: a grant REJECTION is swallowed + logged, never escapes the listener', async () => { + mockSignupGrantService.grantOnSignup.mockRejectedValue(new Error('mongo down')); + const handler = await getHandler(); + // The emit-site catch (organizations) is sync-only — the async listener must resolve, not reject. + await expect(handler(payload)).resolves.toBeUndefined(); + const errCall = mockLogger.error.mock.calls.find(([msg]) => msg.includes('signup grant failed via organization.created listener')); + expect(errCall).toBeDefined(); + expect(errCall[1]).toMatchObject({ orgId: 'org1', planId: 'free', err: 'mongo down' }); + }); + + test('self-guard: even a malformed payload cannot make the listener throw/reject', async () => { + const handler = await getHandler(); + await expect(handler(undefined)).resolves.toBeUndefined(); + expect(mockSignupGrantService.grantOnSignup).toHaveBeenCalledWith({ orgId: undefined, planId: undefined }); + }); + }); + test('resolves without error when meterMode=true and no legacy docs', async () => { mockConfig.billing.meterMode = true; mockConfig.billing.plans = ['free', 'growth', 'pro']; diff --git a/modules/billing/tests/billing.referral.integration.tests.js b/modules/billing/tests/billing.referral.integration.tests.js index df36d4373..9a9cc48a8 100644 --- a/modules/billing/tests/billing.referral.integration.tests.js +++ b/modules/billing/tests/billing.referral.integration.tests.js @@ -103,6 +103,13 @@ describe('Billing referral grant (#3842):', () => { .send({ email: ADMIN_EMAIL, password: PASSWORD }) .expect(200); + // #3952: the admin's own org creation also fires `organization.created` → billing's + // fire-and-forget signupGrant listener. Wait for it to settle here so every downstream + // balance-delta assertion (referral credits) starts from a stable, fully-settled baseline. + const adminOrgId = String(brut.currentOrganization?._id || brut.currentOrganization); + const adminGrant = await waitForLedgerEntry(adminOrgId, `signup_grant-${adminOrgId}`); + expect(adminGrant).not.toBeNull(); + return adminAgent; } @@ -165,6 +172,11 @@ describe('Billing referral grant (#3842):', () => { const inviterBalanceAfter = await BillingExtraBalanceRepository.getBalance(inviterOrgId); expect(inviterBalanceAfter - inviterBalanceBefore).toBe(REFERRER_UNITS); + // #3952: the referee's own org creation also fires `organization.created` → signupGrant. + // Wait for it to settle so the "unchanged after replay" check below (step 5) has a stable + // snapshot — otherwise a late-landing signup grant could be mistaken for a replay double-credit. + const refereeGrant = await waitForLedgerEntry(refereeOrgId, `signup_grant-${refereeOrgId}`); + expect(refereeGrant).not.toBeNull(); const refereeBalanceAfter = await BillingExtraBalanceRepository.getBalance(refereeOrgId); // 4. Replay safety (deterministic): a direct service replay is the exact grant path diff --git a/modules/organizations/lib/events.js b/modules/organizations/lib/events.js index b497c5732..3f521664c 100644 --- a/modules/organizations/lib/events.js +++ b/modules/organizations/lib/events.js @@ -7,6 +7,20 @@ import { EventEmitter } from 'events'; * Singleton emitter for organization events. Config-free / import-safe. * * Events: + * - `organization.created` — emitted (#3952) by BOTH org-creation call sites + * (organizations.crud.service.js::create, the generic POST /api/organizations path, AND + * organizations.service.js::createOrganizationForUser, the signup path) immediately after + * the organization + owner membership are durably created. This is the sanctioned seam for + * billing's one-shot signupGrant (billing.init.js), replacing a direct import of + * BillingSignupGrantService from organizations — the organizations module must stay + * removable without billing. Fire-and-forget; the emit-site try/catch only guards a + * SYNCHRONOUS listener throw — see the `organization.provisioned` note below for the + * async-rejection caveat, which applies identically here (the listener owns its own guard). + * Payload: { + * orgId: String — the freshly created organization's id + * planId: String — the plan to evaluate the grant against (both call sites pass 'free' — + * the only plan a fresh org can have at creation time) + * } * - `organization.provisioned` — emitted (#3844) by OrganizationsService.handleSignupOrganization * on EVERY exit path that returns a real organization: the fresh-create paths AND the A4 * idempotent-convergence path (downstream consumers must be idempotent — a converged retry diff --git a/modules/organizations/services/organizations.crud.service.js b/modules/organizations/services/organizations.crud.service.js index 9f109b1e9..a68143331 100644 --- a/modules/organizations/services/organizations.crud.service.js +++ b/modules/organizations/services/organizations.crud.service.js @@ -23,7 +23,7 @@ const normalizeDomain = (value = '') => value.trim().toLowerCase(); import OrganizationsRepository from '../repositories/organizations.repository.js'; import MembershipRepository from '../repositories/organizations.membership.repository.js'; import UserService from '../../users/services/users.service.js'; -import BillingSignupGrantService from '../../billing/services/billing.signupGrant.service.js'; +import organizationEvents from '../lib/events.js'; import { slugify } from '../helpers/organizations.slug.js'; import { MEMBERSHIP_STATUSES, MEMBERSHIP_ROLES } from '../lib/constants.js'; import { runOrganizationRemovedHandlers } from '../lib/orgRemoval.registry.js'; @@ -121,13 +121,17 @@ const create = async (body, user) => { throw err; } - // Best-effort signup grant — mirrors organizations.service.js::createOrganizationForUser. - // Every org-creation path (including this generic one behind POST /api/organizations) - // must credit the configured one-shot signupGrant, else a fresh org on a plan that - // defines one starts at 0 balance. Called outside the rollback try/catch so a billing - // failure never rolls back the org; grantOnSignup never throws and is idempotent - // (refId signup_grant-). No-op when the plan defines no signupGrant. - await BillingSignupGrantService.grantOnSignup({ orgId: result._id.toString(), planId: result.plan || 'free' }); + // organization.created (#3952) — mirrors organizations.service.js::createOrganizationForUser. + // Billing subscribes from billing.init.js and credits the configured one-shot signupGrant, + // else a fresh org on a plan that defines one starts at 0 balance. Emitted outside the + // rollback try/catch so a billing failure never rolls back the org; the listener owns its + // idempotence (refId signup_grant-) and never throws. Fire-and-forget, synchronous + // emit — the try/catch here only guards a SYNCHRONOUS listener throw (see ../lib/events.js). + try { + organizationEvents.emit('organization.created', { orgId: result._id.toString(), planId: result.plan || 'free' }); + } catch (err) { + logger.warn('organizations: organization.created listener threw', { message: err?.message }); + } return result; }; diff --git a/modules/organizations/services/organizations.service.js b/modules/organizations/services/organizations.service.js index a1d3a5790..d4246280f 100644 --- a/modules/organizations/services/organizations.service.js +++ b/modules/organizations/services/organizations.service.js @@ -12,7 +12,6 @@ import UserService from '../../users/services/users.service.js'; import { slugify, generateOrganizationSlug } from '../helpers/organizations.slug.js'; import { MEMBERSHIP_ROLES, MEMBERSHIP_STATUSES } from '../lib/constants.js'; import organizationEvents from '../lib/events.js'; -import BillingSignupGrantService from '../../billing/services/billing.signupGrant.service.js'; import { isPublicDomain, normalizeEmailDomain } from './organizations.domain.js'; /** @@ -96,9 +95,17 @@ const createOrganizationForUser = async ({ name, slug, domain, user, slugGenerat } if (created) { - // N2: best-effort signup grant — called outside the try/catch so billing failure - // never triggers org/membership rollback. grantOnSignup always resolves (never throws). - await BillingSignupGrantService.grantOnSignup({ orgId: organization._id.toString(), planId: 'free' }); + // organization.created (#3952) — N2 signup grant moved off a direct billing import onto + // this event; billing subscribes from billing.init.js and credits the one-shot signupGrant. + // Emitted outside the try/catch above so billing failure never triggers org/membership + // rollback; the listener owns its own guard (never throws/rejects out). Fire-and-forget, + // synchronous emit — this try/catch only guards a SYNCHRONOUS listener throw (mirrors + // emitProvisioned below / see ../lib/events.js). + try { + organizationEvents.emit('organization.created', { orgId: organization._id.toString(), planId: 'free' }); + } catch (err) { + logger.warn('organizations: organization.created listener threw', { message: err?.message }); + } return { organization, membership }; } } @@ -124,8 +131,9 @@ const createOrganizationForUser = async ({ name, slug, domain, user, slugGenerat * If domainMatching is on, the user's email domain is not public, and an existing org matches, * a `suggestedJoin: { orgId, orgName }` hint (name-only) is returned alongside the new org. * - * signupGrant: credited inside createOrganizationForUser (best-effort, never throws). Called - * exactly once per real new org — no double-credit possible. + * signupGrant: createOrganizationForUser emits `organization.created` (#3952) exactly once per + * real new org — no double-credit possible; billing's subscriber (billing.init.js) credits the + * grant asynchronously, best-effort, never throwing back into this flow. * * @param {Object} user - The user object returned by UserService.create (with id, email, firstName, lastName). * @returns {Promise<{organization: Object|null, membership: Object|null, abilities: Array, emailVerificationRequired?: boolean, suggestedJoin?: {orgId: string, orgName: string}}>} diff --git a/modules/organizations/tests/organizations.crud.grant.unit.tests.js b/modules/organizations/tests/organizations.crud.grant.unit.tests.js index aa747a07d..763b45bde 100644 --- a/modules/organizations/tests/organizations.crud.grant.unit.tests.js +++ b/modules/organizations/tests/organizations.crud.grant.unit.tests.js @@ -4,15 +4,17 @@ import { jest, describe, test, expect, beforeEach } from '@jest/globals'; /** - * Unit tests — verify organizations.crud.service.create() credits the one-shot - * signup grant on the generic POST /api/organizations path, mirroring the - * invite/verify path (organizations.service.js::createOrganizationForUser). A - * fresh org on a plan that defines a signupGrant must not start at 0 balance. + * Unit tests — verify organizations.crud.service.create() emits `organization.created` on + * the generic POST /api/organizations path, mirroring the invite/verify path + * (organizations.service.js::createOrganizationForUser). Billing subscribes to this event + * from billing.init.js and credits the one-shot signupGrant (#3952) — organizations no + * longer imports the billing module directly (see modules/billing/tests/billing.init.unit.tests.js + * for the subscriber-side coverage). */ -const mockGrantOnSignup = jest.fn().mockResolvedValue({ applied: true }); -jest.unstable_mockModule('../../billing/services/billing.signupGrant.service.js', () => ({ - default: { grantOnSignup: mockGrantOnSignup }, +const mockEmit = jest.fn(); +jest.unstable_mockModule('../lib/events.js', () => ({ + default: { emit: mockEmit, on: jest.fn() }, })); const mockOrgCreate = jest.fn().mockResolvedValue({ _id: 'org1', plan: 'free' }); @@ -64,19 +66,19 @@ jest.unstable_mockModule('../../../config/index.js', () => ({ const { default: OrgCrudService } = await import('../services/organizations.crud.service.js'); -describe('organizations.crud.service.create signup grant:', () => { +describe('organizations.crud.service.create organization.created emit:', () => { beforeEach(() => { - mockGrantOnSignup.mockClear(); + mockEmit.mockClear(); mockOrgCreate.mockResolvedValue({ _id: 'org1', plan: 'free' }); }); - test('credits the one-shot signup grant for the freshly created org', async () => { + test('emits organization.created for the freshly created org', async () => { const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true }; const result = await OrgCrudService.create({ name: 'Test Org' }, user); expect(result).toEqual({ _id: 'org1', plan: 'free' }); - expect(mockGrantOnSignup).toHaveBeenCalledTimes(1); - expect(mockGrantOnSignup).toHaveBeenCalledWith({ orgId: 'org1', planId: 'free' }); + expect(mockEmit).toHaveBeenCalledTimes(1); + expect(mockEmit).toHaveBeenCalledWith('organization.created', { orgId: 'org1', planId: 'free' }); }); test('defaults planId to free when the created org has no plan field', async () => { @@ -84,14 +86,23 @@ describe('organizations.crud.service.create signup grant:', () => { const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true }; await OrgCrudService.create({ name: 'No Plan Org' }, user); - expect(mockGrantOnSignup).toHaveBeenCalledWith({ orgId: 'org2', planId: 'free' }); + expect(mockEmit).toHaveBeenCalledWith('organization.created', { orgId: 'org2', planId: 'free' }); }); - test('does not credit the grant when membership creation fails (rollback exits before the grant)', async () => { + test('does not emit when membership creation fails (rollback exits before the emit)', async () => { mockMembershipCreate.mockRejectedValueOnce(new Error('membership boom')); const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true }; await expect(OrgCrudService.create({ name: 'Rollback Org' }, user)).rejects.toThrow(); - expect(mockGrantOnSignup).not.toHaveBeenCalled(); + expect(mockEmit).not.toHaveBeenCalled(); + }); + + test('a SYNCHRONOUS listener throw is swallowed — org creation still succeeds', async () => { + mockEmit.mockImplementationOnce(() => { throw new Error('listener exploded'); }); + const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true }; + + const result = await OrgCrudService.create({ name: 'Test Org' }, user); + + expect(result).toEqual({ _id: 'org1', plan: 'free' }); }); }); diff --git a/modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js b/modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js index a86200afd..c8ba5aa9f 100644 --- a/modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js +++ b/modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js @@ -88,11 +88,6 @@ jest.unstable_mockModule('../helpers/organizations.slug.js', () => ({ generateOrganizationSlug: jest.fn().mockResolvedValue('test-slug'), })); -const mockBillingGrantOnSignup = jest.fn().mockResolvedValue(undefined); -jest.unstable_mockModule('../../billing/services/billing.signupGrant.service.js', () => ({ - default: { grantOnSignup: mockBillingGrantOnSignup }, -})); - // --- Dynamic imports after mocks --- const { default: OrganizationsService } = await import('../services/organizations.service.js'); diff --git a/modules/organizations/tests/organizations.integration.tests.js b/modules/organizations/tests/organizations.integration.tests.js index 78fed89f1..d530be89d 100644 --- a/modules/organizations/tests/organizations.integration.tests.js +++ b/modules/organizations/tests/organizations.integration.tests.js @@ -149,6 +149,26 @@ describe('Organizations integration tests:', () => { let grantUser; let BillingExtraBalanceRepository; + /** + * Poll the org ledger until a signup_grant entry appears. #3952 moved the grant off a + * direct synchronous call onto the `organization.created` event (organizations/lib/events.js) — + * billing's subscriber (billing.init.js) is fire-and-forget, so the signup response does not + * await it; the credit lands shortly after, not necessarily before the HTTP response returns. + * @param {string} orgId - Organization id whose ledger to poll. + * @param {number} [timeoutMs=5000] - Give-up timeout. + * @returns {Promise} The ledger entry, or null on timeout. + */ + async function waitForSignupGrantEntry(orgId, timeoutMs = 5000) { + const start = Date.now(); + while (Date.now() - start < timeoutMs) { + const ledger = await BillingExtraBalanceRepository.findLedgerByOrg(orgId); + const entry = (ledger ?? []).find((e) => e.source === 'signup_grant'); + if (entry) return entry; + await new Promise((resolve) => { setTimeout(resolve, 100); }); + } + return null; + } + beforeAll(async () => { config.organizations = { enabled: false }; BillingExtraBalanceRepository = (await import(path.resolve('./modules/billing/repositories/billing.extraBalance.repository.js'))).default; @@ -173,14 +193,12 @@ describe('Organizations integration tests:', () => { expect(memberships.length).toBeGreaterThan(0); const orgId = (memberships[0].organizationId._id || memberships[0].organizationId).toString(); + const grantEntry = await waitForSignupGrantEntry(orgId); + expect(grantEntry).not.toBeNull(); + expect(grantEntry.amount).toBe(500); + const balance = await BillingExtraBalanceRepository.getBalance(orgId); expect(balance).toBe(500); - - const ledger = await BillingExtraBalanceRepository.findLedgerByOrg(orgId); - expect(ledger).not.toBeNull(); - const grantEntry = ledger.find((e) => e.source === 'signup_grant'); - expect(grantEntry).toBeDefined(); - expect(grantEntry.amount).toBe(500); }); afterAll(async () => { diff --git a/modules/organizations/tests/organizations.service.signup.unit.tests.js b/modules/organizations/tests/organizations.service.signup.unit.tests.js index 9414bf60f..555407c7b 100644 --- a/modules/organizations/tests/organizations.service.signup.unit.tests.js +++ b/modules/organizations/tests/organizations.service.signup.unit.tests.js @@ -7,8 +7,8 @@ * - `suggestedJoin` returned ONLY when: domainMatching on + non-public domain + existing different org matches. * Shape is name-only: { orgId: string, orgName: string } — no size/domain/membership keys. * - Public domain (e.g. gmail.com) with same-domain existing org → NO suggestedJoin. - * - signupGrant credited exactly once per real new org creation (via BillingSignupGrantService.grantOnSignup); - * not double-credited on any path. + * - `organization.created` (#3952) emitted exactly once per real new org creation (billing's + * subscriber in billing.init.js owns crediting the grant); not double-emitted on any path. * * All assertions pass with the A2 implementation shipped in this PR. */ @@ -17,11 +17,6 @@ import { jest, describe, test, expect, beforeEach } from '@jest/globals'; // --- Mocks (must precede dynamic imports) --- -const mockGrantOnSignup = jest.fn().mockResolvedValue(null); -jest.unstable_mockModule('../../billing/services/billing.signupGrant.service.js', () => ({ - default: { grantOnSignup: mockGrantOnSignup }, -})); - const mockIsConfigured = jest.fn().mockReturnValue(false); jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({ default: { isConfigured: mockIsConfigured }, @@ -80,13 +75,20 @@ jest.unstable_mockModule('../../../lib/services/logger.js', () => ({ default: { error: jest.fn(), warn: jest.fn(), info: jest.fn() }, })); -// #3844: capture organization.provisioned emits — the singleton is config-free, stub it -// so assertions see the emit without wiring real listeners. +// #3844/#3952: capture organization.provisioned + organization.created emits — the singleton +// is config-free, stub it so assertions see the emits without wiring real listeners. const mockOrgEventsEmit = jest.fn(); jest.unstable_mockModule('../lib/events.js', () => ({ default: { emit: mockOrgEventsEmit, on: jest.fn() }, })); +/** + * Filter captured `organizationEvents.emit` calls down to one event name's payloads. + * @param {string} eventName - e.g. 'organization.created'. + * @returns {Array} the payload of each matching emit call, in call order. + */ +const emittedPayloads = (eventName) => mockOrgEventsEmit.mock.calls.filter(([name]) => name === eventName).map(([, payload]) => payload); + // Config store — MUST be mutated in-place (not reassigned) because jest.unstable_mockModule // captures the default export value at import time. Object.assign ensures live updates. const configStore = { organizations: {} }; @@ -169,7 +171,6 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { mockIsConfigured.mockReturnValue(false); mockOrgExists.mockResolvedValue(false); mockUpdateById.mockResolvedValue({}); - mockGrantOnSignup.mockResolvedValue(null); // Default: no active membership exists (fresh user — normal always-create path) mockMembershipFindOne.mockResolvedValue(null); // Re-establish policy + abilities mocks wiped by resetAllMocks @@ -316,22 +317,21 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { }); }); - // ─── signupGrant — credited exactly once per real new org ──────────────── + // ─── organization.created (#3952) — emitted exactly once per real new org ─ - describe('signupGrant (BillingSignupGrantService):', () => { - test('credited exactly once for a real new signup (autoCreate:false, no domain match)', async () => { + describe('organization.created emit (billing subscribes from billing.init.js):', () => { + test('emitted exactly once for a real new signup (autoCreate:false, no domain match)', async () => { setupConfig({ enabled: true, autoCreate: false, domainMatching: false }); const user = makeUser('alice@acme.com'); await OrganizationsService.handleSignupOrganization(user); - expect(mockGrantOnSignup).toHaveBeenCalledTimes(1); - expect(mockGrantOnSignup).toHaveBeenCalledWith( - expect.objectContaining({ planId: 'free' }), - ); + const created = emittedPayloads('organization.created'); + expect(created).toHaveLength(1); + expect(created[0]).toEqual(expect.objectContaining({ planId: 'free' })); }); - test('credited exactly once for a real new signup (autoCreate:true, domain match → always-create)', async () => { + test('emitted exactly once for a real new signup (autoCreate:true, domain match → always-create)', async () => { setupConfig({ enabled: true, autoCreate: true, domainMatching: true }); const existingOrg = makeFakeOrg({ name: 'Acme Corp', domain: 'acme.com' }); mockOrgList.mockResolvedValue([existingOrg]); @@ -339,11 +339,11 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { await OrganizationsService.handleSignupOrganization(user); - // One grant for the newly created workspace (not for the existing org) - expect(mockGrantOnSignup).toHaveBeenCalledTimes(1); + // One event for the newly created workspace (not for the existing org) + expect(emittedPayloads('organization.created')).toHaveLength(1); }); - test('NOT called on email-verification early-return path', async () => { + test('NOT emitted on email-verification early-return path', async () => { setupConfig({ enabled: true, autoCreate: false, domainMatching: false }); mockIsConfigured.mockReturnValue(true); const user = makeUser('alice@acme.com'); @@ -351,9 +351,9 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { const result = await OrganizationsService.handleSignupOrganization(user); - // Early return — no org created, no grant + // Early return — no org created, no emit expect(result.organization).toBeNull(); - expect(mockGrantOnSignup).not.toHaveBeenCalled(); + expect(emittedPayloads('organization.created')).toHaveLength(0); }); }); @@ -483,7 +483,7 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { // GREEN: guard detects existing active membership → converges to it, skips create. describe('A4 — idempotent convergence (retry-safe signup provisioning):', () => { - test('user already has active membership → returns existing org+membership, NO new org created, NO grant', async () => { + test('user already has active membership → returns existing org+membership, NO new org created, NO organization.created emit', async () => { setupConfig({ enabled: true, autoCreate: false, domainMatching: false }); const existingOrg = makeFakeOrg({ name: 'Existing Corp', domain: 'acme.com' }); const existingMembership = { @@ -504,7 +504,7 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { // No duplicate org created expect(mockOrgCreate).not.toHaveBeenCalled(); // No double-credit on convergence path - expect(mockGrantOnSignup).not.toHaveBeenCalled(); + expect(emittedPayloads('organization.created')).toHaveLength(0); // No footgun keys expect(result).not.toHaveProperty('organizationSetupRequired'); expect(result).not.toHaveProperty('pendingJoin'); @@ -527,10 +527,10 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { expect(result.organization).toBe(existingOrg); expect(result.membership).toBe(existingMembership); expect(mockOrgCreate).not.toHaveBeenCalled(); - expect(mockGrantOnSignup).not.toHaveBeenCalled(); + expect(emittedPayloads('organization.created')).toHaveLength(0); }); - test('genuinely new user (no existing membership) → org IS created, grant credited once', async () => { + test('genuinely new user (no existing membership) → org IS created, organization.created emitted once', async () => { setupConfig({ enabled: true, autoCreate: false, domainMatching: false }); // No active membership exists (default from beforeEach) mockMembershipFindOne.mockResolvedValue(null); @@ -541,7 +541,7 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { expect(result.organization).not.toBeNull(); expect(result.membership).not.toBeNull(); expect(mockOrgCreate).toHaveBeenCalledTimes(1); - expect(mockGrantOnSignup).toHaveBeenCalledTimes(1); + expect(emittedPayloads('organization.created')).toHaveLength(1); }); test('converge path returns abilities from policy (same shape as fresh-signup path)', async () => { @@ -579,7 +579,7 @@ describe('handleSignupOrganization — always-create (spec D5 / A2):', () => { expect(result.emailVerificationRequired).toBe(true); expect(mockMembershipFindOne).not.toHaveBeenCalled(); expect(mockOrgCreate).not.toHaveBeenCalled(); - expect(mockGrantOnSignup).not.toHaveBeenCalled(); + expect(emittedPayloads('organization.created')).toHaveLength(0); }); test('converge path does NOT return suggestedJoin (retry, not fresh corporate signup)', async () => { @@ -603,13 +603,12 @@ describe('handleSignupOrganization — organization.provisioned emit (#3844):', mockIsConfigured.mockReturnValue(false); mockOrgExists.mockResolvedValue(false); mockUpdateById.mockResolvedValue({}); - mockGrantOnSignup.mockResolvedValue(null); mockMembershipFindOne.mockResolvedValue(null); mockDefineAbilityFor.mockResolvedValue({ rules: [] }); mockSerializeAbilities.mockReturnValue(['ability-stub']); }); - test('fresh-create path → emits ONCE with string userId + organizationId', async () => { + test('fresh-create path → emits organization.created THEN organization.provisioned, each once', async () => { setupConfig({ enabled: true }); const fakeOrg = makeFakeOrg(); mockOrgCreate.mockResolvedValue(fakeOrg); @@ -618,8 +617,15 @@ describe('handleSignupOrganization — organization.provisioned emit (#3844):', const result = await OrganizationsService.handleSignupOrganization(user); expect(result.organization).not.toBeNull(); - expect(mockOrgEventsEmit).toHaveBeenCalledTimes(1); - expect(mockOrgEventsEmit).toHaveBeenCalledWith('organization.provisioned', { + // Two events on a genuinely new org: organization.created (#3952, billing's signupGrant + // seam) fires inside createOrganizationForUser, THEN organization.provisioned (#3844) + // fires back in handleSignupOrganization. + expect(mockOrgEventsEmit).toHaveBeenCalledTimes(2); + expect(mockOrgEventsEmit).toHaveBeenNthCalledWith(1, 'organization.created', { + orgId: String(fakeOrg._id), + planId: 'free', + }); + expect(mockOrgEventsEmit).toHaveBeenNthCalledWith(2, 'organization.provisioned', { userId: String(user.id), organizationId: String(fakeOrg._id), }); @@ -640,6 +646,8 @@ describe('handleSignupOrganization — organization.provisioned emit (#3844):', expect(result.organization).toBe(existingOrg); expect(mockOrgCreate).not.toHaveBeenCalled(); + // No new org → no organization.created (that only fires from createOrganizationForUser, + // which the convergence path never calls); provisioned still fires for the converged org. expect(mockOrgEventsEmit).toHaveBeenCalledTimes(1); expect(mockOrgEventsEmit).toHaveBeenCalledWith('organization.provisioned', { userId: String(user.id),