Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain across checkout, seat enforcement, concurrency, membership flows, synchronization, and related UI.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds Stripe-backed purchased seats and tiered pricing for Team plans, with enforcement across membership flows, seat management, auditing, and backfill support.
Changes:
- Adds seat accounting, reservations, locking, enforcement, and audit logging.
- Adds tiered Stripe pricing, checkout, previews, prorated increases, and scheduled decreases.
- Updates billing UI, shared pricing copy, documentation, tests, and deployment tooling.
File summaries
| File | Reviewed change |
|---|---|
tsconfig.json |
Updates TypeScript project references. |
tsconfig.base.json |
Updates shared compiler configuration. |
prisma/schema.prisma |
Adds purchased-seat and pending-decrease fields. |
prisma/migrations/20260910151418_team_purchased_seats/migration.sql |
Creates billing-account seat columns. |
libs/ui/src/lib/form/input/NumberStepperInput.tsx |
Adds reusable seat-count input. |
libs/ui/src/lib/form/input/__tests__/NumberStepperInput.spec.tsx |
Tests stepper behavior. |
libs/ui/src/index.ts |
Exports the stepper component. |
libs/types/src/lib/team.types.ts |
Adds team seat types. |
libs/types/src/lib/billing.types.ts |
Adds tier pricing and checkout types. |
libs/test/e2e-utils/src/lib/TeamCreationUtils.ts |
Adds seat-aware E2E fixtures. |
libs/test/e2e-utils/src/lib/pageObjectModels/TeamDashboardPage.model.ts |
Adds dashboard seat workflow support. |
libs/team-seats/vite.config.mts |
Configures team-seats tooling. |
libs/team-seats/tsconfig.spec.json |
Configures test compilation. |
libs/team-seats/tsconfig.lib.json |
Configures library compilation. |
libs/team-seats/tsconfig.json |
Adds project compiler settings. |
libs/team-seats/src/lib/seat-math.ts |
Centralizes seat calculations. |
libs/team-seats/src/lib/seat-enforcement.ts |
Enforces seat availability. |
libs/team-seats/src/lib/membership.ts |
Handles invitation membership conversion. |
libs/team-seats/src/lib/__tests__/seat-math.spec.ts |
Tests seat calculations. |
libs/team-seats/src/lib/__tests__/seat-enforcement.spec.ts |
Tests seat enforcement. |
libs/team-seats/src/index.ts |
Exports team-seat functionality. |
libs/team-seats/project.json |
Registers project targets. |
libs/shared/utils/src/lib/team-seat-pricing.ts |
Defines shared seat pricing calculations. |
libs/shared/utils/src/index.ts |
Exports shared utilities. |
libs/shared/ui-utils/src/lib/billing-format-utils.ts |
Adds billing and price formatting helpers. |
libs/shared/ui-utils/src/index.ts |
Exports shared UI utilities. |
libs/shared/data/src/lib/client-data.ts |
Updates client billing data access. |
libs/shared/constants/src/lib/shared-constants.ts |
Updates shared application constants. |
libs/shared/constants/src/lib/pricing-constants.ts |
Shares Team pricing and feature copy. |
libs/shared/constants/src/index.ts |
Exports shared constants. |
libs/features/teams/src/lib/TeamDashboard/TeamMemberUpdateModal.tsx |
Integrates seat-aware role updates. |
libs/features/teams/src/lib/TeamDashboard/TeamMemberStatusUpdateModal.tsx |
Integrates seat-aware status changes. |
libs/features/teams/src/lib/TeamDashboard/TeamMemberInviteModal.tsx |
Adds seat-aware invitations. |
libs/features/teams/src/lib/TeamDashboard/TeamDashboard.tsx |
Integrates seat summaries and actions. |
libs/features/teams/src/lib/TeamDashboard/TeamAuditLogModal.tsx |
Displays team audit events. |
libs/features/teams/src/lib/TeamDashboard/team-seats/TeamSeatsManageModal.tsx |
Provides seat preview and commit flow. |
libs/features/teams/src/lib/TeamDashboard/team-seats/TeamSeats.tsx |
Displays seat usage and limits. |
libs/features/teams/src/lib/TeamDashboard/team-seats/team-seats.utils.ts |
Provides seat-management UI helpers. |
libs/features/teams/src/lib/TeamDashboard/team-seats/SeatsUnavailableNotice.tsx |
Displays unavailable-seat guidance. |
libs/features/teams/src/lib/TeamDashboard/team-seats/__tests__/team-seats.utils.spec.ts |
Tests seat UI helpers. |
libs/features/teams/src/lib/TeamDashboard/team-members/TeamMembersTable.tsx |
Integrates seat-related member actions. |
libs/auth/server/tsconfig.lib.json |
Updates auth server compilation. |
libs/auth/server/src/lib/sso-auth.service.ts |
Enforces seats during SSO provisioning. |
libs/auth/server/src/lib/__tests__/sso-auth.service.spec.ts |
Tests SSO seat enforcement. |
libs/auth/server/src/lib/__tests__/auth.db.service.team-join.spec.ts |
Tests team-join seat enforcement. |
libs/auth/acl/src/lib/acl.ts |
Adds seat-management permissions. |
libs/auth/acl/src/lib/__tests__/acl.spec.ts |
Tests seat authorization. |
libs/audit-logs/src/lib/audit-logs.ts |
Adds audit-log support for blocked changes. |
apps/landing/pages/privacy/index.tsx |
Updates landing privacy content. |
apps/landing/pages/pricing/index.tsx |
Updates public Team pricing. |
apps/landing/pages/dpa/index.tsx |
Updates DPA content. |
apps/landing/components/Soc2Badge.tsx |
Updates the compliance badge component. |
apps/landing/components/landing/PricingPreview.tsx |
Updates the pricing preview. |
apps/landing/components/landing/LandingPage.tsx |
Integrates updated landing content. |
apps/landing/components/landing/landing-page-data.ts |
Updates landing-page data. |
apps/landing/components/Footer.tsx |
Updates footer content and links. |
apps/jetstream/src/app/components/billing/TeamNameModal.tsx |
Updates Team billing setup. |
apps/jetstream/src/app/components/billing/TeamCheckoutOptions.tsx |
Adds seat-aware checkout options. |
apps/jetstream/src/app/components/billing/EnhancedBillingCard.tsx |
Displays billing and seat details. |
apps/jetstream/src/app/components/billing/BillingPeriodToggle.tsx |
Updates billing-period selection. |
apps/jetstream/src/app/components/billing/billing.utils.ts |
Adds billing and seat helpers. |
apps/jetstream/src/app/components/billing/Billing.tsx |
Integrates seat selection into checkout. |
apps/jetstream/src/app/components/billing/billing.constants.ts |
Updates billing UI constants. |
apps/jetstream/src/app/components/billing/__tests__/billing.utils.spec.ts |
Tests billing utilities. |
apps/docs/docs/user-profile-and-settings/billing.mdx |
Documents billing changes. |
apps/docs/docs/team-management/team-management.mdx |
Documents Team seat management. |
apps/cron-tasks/src/config/env-config.ts |
Adds backfill configuration. |
apps/cron-tasks/src/backfill-team-seats.ts |
Adds the backfill command. |
apps/cron-tasks/project.json |
Registers the backfill target. |
apps/api/tsconfig.app.json |
Updates API compiler configuration. |
apps/api/src/app/utils/error-handler.ts |
Updates API error handling. |
apps/api/src/app/services/team.service.ts |
Applies seat-aware team operations. |
apps/api/src/app/services/__tests__/team.service.spec.ts |
Tests team service behavior. |
apps/api/src/app/routes/team.routes.ts |
Adds or updates Team API routes. |
apps/api/src/app/routes/openapi.routes.ts |
Documents Team API endpoints. |
apps/api/src/app/controllers/billing.controller.ts |
Adds Team checkout and billing endpoints. |
Review details
Suppressed comments (6)
apps/api/src/app/controllers/billing.controller.ts:207
- For tiered Team subscriptions,
convertCustomerWithSubscriptionsToUserFacingcannot see Stripe'stiersfrom the expanded subscription item, so it returnstiers: nullhere. The billing UI uses those tiers to calculate the active subscription total/rate; because this handler bypassesgetUserFacingStripeCustomer/attachTiersToTieredItems, every existing tiered Team subscription renders unknown pricing despitepricesByLookupKeybeing loaded. Attach the tier tables before sendingcustomer.
const pricesByLookupKey = await stripeService.fetchPrices({ lookupKeys: STRIPE_PRICE_KEYS });
apps/api/src/app/controllers/team.controller.ts:837
- This route also handles an active BILLING member being changed to a billable role, which is a role change rather than a reactivation. Because the audit context always records
REACTIVATE, blocked role changes through the status-and-role endpoint are mislabeled in the team audit log, making the new seat-denial trail inaccurate. Derive the attempted action from the actual current/requested membership transition.
attemptedAction: 'REACTIVATE',
apps/api/src/app/controllers/team.controller.ts:620
- The blocked-seat audit context hardcodes
role: MEMBER, butacceptTeamInvitationcan be accepting an ADMIN invitation (and the seat check uses the invitation's actual role). When that acceptance is rejected, the audit entry records the wrong attempted role, making the new seat-block audit trail inaccurate. Propagate the invitation role into the rejection/audit context instead of using this constant.
role: TEAM_MEMBER_ROLE_MEMBER,
libs/features/teams/src/lib/TeamDashboard/TeamDashboard.tsx:100
- The generic
ability.can('update', 'TeamSeats')ignores the ACL condition that denies updates forbillingStatus: PAST_DUE(libs/auth/acl/src/lib/acl.ts:113-115). ThiscanManageSeatsvalue is reused by the dashboard banner and member-modal “Buy more seats” links, so past-due users get enabled entry points even though only the card button is disabled; clicking them opens a flow that always fails withSEATS_PAST_DUE. Check the team-scoped subject or guard every entry point before exposing these actions.
libs/features/teams/src/lib/TeamDashboard/team-seats/TeamSeatsManageModal.tsx:49 - The server rejects any requested count below
context.seatState.includedSeatsfor legacy flat-first-tier prices, but this modal's minimum only considers current usage. A legacy team with five included seats can decrement the stepper below five and gets an error for every such preview instead of being constrained to the valid range. Pass the included-seat minimum into the modal (or expose it in the seat summary) and include it inminSeats.
libs/ui/src/lib/form/input/NumberStepperInput.tsx:27 parseIntaccepts numeric prefixes rather than requiring the whole input to be an integer, so typing3.5emits3immediately (and4e2emits4). Thestep=1attribute only marks a decimal as invalid; it does not prevent the change event, so the component silently truncates user input instead of preserving it until blur. Parse the complete string withNumberafter handling the empty string.
- Files reviewed: 90/91 changed files
- Comments generated: 9
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Went through Copilot's suppressed (low-confidence) comments — most were valid:
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain in checkout authorization, billing integrity, membership expiration, audit accuracy, and seat-management gating.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
apps/api/src/app/controllers/billing.controller.ts:150
- When an existing team has no billing account yet, this fallback reuses the caller's personal Stripe customer. Completing the Team checkout then changes that customer's metadata to
TEAMand attaches the Team subscription alongside any individual subscription, so later synchronization can attribute personal billing to the team. Leave this undefined when the team has no customer socreateCheckoutSessioncreates a dedicated Team customer.
customerId: team?.billingAccount?.customerId ?? user.billingAccount?.customerId,
apps/api/src/app/controllers/team.controller.ts:845
- For reactivation,
roleis optional and the existing inactive member may be anADMIN. When that reactivation is blocked for lack of a seat, this fallback recordsMEMBERinstead of the actual role, making the audit trail inaccurate. Use the member's role from the locked read when the request omitsrole.
role: role || TEAM_MEMBER_ROLE_MEMBER,
apps/landing/components/Soc2Badge.tsx:28
- This branch also adds a SOC 2 badge and expands the landing-page, privacy-policy, and DPA content, but the PR description only describes purchased-seat billing. Please either document these additional externally visible/legal changes and review them as part of this release, or split them into a separate PR so the seat rollout has an unambiguous scope.
- Files reviewed: 90/91 changed files
- Comments generated: 5
- Review effort level: Lite
|
Second pass over Copilot's suppressed comments:
|
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Pull request overview
Copilot reviewed 90 out of 91 changed files in this pull request and generated no new comments.
Suppressed comments (3)
libs/ui/src/lib/form/input/NumberStepperInput.tsx:1
- This component calls
setStateduring render (inside the function body). Even with the guard, React treats state updates during render as an anti-pattern and it can produce warnings/errors in StrictMode or lead to hard-to-debug re-render behavior. Move this sync logic into auseEffectkeyed onvalue(and any other relevant inputs) so state is updated after render.
libs/shared/utils/src/lib/team-seat-pricing.ts:1 - The
!firstTier.flatAmountcheck treatsflatAmount = 0as 'missing', which would incorrectly return 0 included seats for a valid tier that includes seats at a $0 flat amount. Prefer an explicit null/undefined check (e.g.firstTier.flatAmount == null) so0is handled correctly.
libs/features/teams/src/lib/TeamDashboard/TeamDashboard.tsx:1 - The ACL rules for
TeamSeatsupdates include conditionalcannotclauses (manualBilling,billingStatus). Checking with just the subject string (ability.can('update', 'TeamSeats')) won’t evaluate those field-based conditions, so the UI can diverge from the intended permission model. Use a subject object when checking (e.g.{ type: 'TeamSeats', manualBilling: hasManualBilling, billingStatus: team?.billingStatus }) and derivecanManageSeatsfrom that.
4c19d89 to
08bf971
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved billing checkout, subscription-state, synchronization, and UI issues block approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
apps/api/src/app/services/team-seats.service.ts:482
- The Stripe mutation has already succeeded before this call, so propagating a transient synchronization error makes a paid seat change return as a failure while the database still shows the old cap. The admin can retry and potentially submit the same change again, and the UI receives no successful result even though Stripe has applied it. Treat post-success mirroring as recoverable (log/retry it and return a success or an explicit synchronization-pending result) rather than turning the completed billing operation into an error.
await resynchronizeSeatState(context);
apps/jetstream/src/app/components/billing/Billing.tsx:365
- When a user who is already a team member has the MEMBER role,
getSubscriptionsdeliberately returns no team data andteamremains null inBilling. This Team card is still selectable (and is the default plan for any team member), so the user is shown a new-team seat/name checkout that the API will reject becausecreateCheckoutSessionHandlerforbids non-admin/non-billing members. Disable or hide the Team option for existing users who cannot manage team billing, rather than allowing an inevitably failing checkout.
pricingTiers={
isAnnual ? PLAN_DESCRIPTIONS[TEAM_ANNUAL_KEY].pricingTiers : PLAN_DESCRIPTIONS[TEAM_MONTHLY_KEY].pricingTiers
}
checked={
selectedPlan === (isAnnual ? PLAN_DESCRIPTIONS[TEAM_ANNUAL_KEY].key : PLAN_DESCRIPTIONS[TEAM_MONTHLY_KEY].key)
}
value={isAnnual ? PLAN_DESCRIPTIONS[TEAM_ANNUAL_KEY].key : PLAN_DESCRIPTIONS[TEAM_MONTHLY_KEY].key}
onChange={setSelectedPlan}
- Files reviewed: 86/87 changed files
- Comments generated: 2
- Review effort level: Lite
|
Went through Copilot's suppressed (low-confidence) comments across all four reviews — one was valid:
The rest were already addressed on the branch or did not apply: tiers are attached via |
08bf971 to
1720334
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Seven unresolved findings remain, including three critical synchronization/backfill issues and four moderate correctness/concurrency issues.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
apps/api/src/app/controllers/team.controller.ts:620
- A blocked acceptance of an ADMIN invitation is logged with
role: MEMBERbecause this wrapper hard-codes the role before the service reads the invitation. The audit entry therefore misidentifies the membership operation; pass the invitation's actual role through the acceptance result/error context (including ADMIN) so the audit log accurately records who was denied.
attemptedAction: 'ACCEPT_INVITATION',
resource: AuditLogResource.TEAM_MEMBER,
resourceId: user.id,
role: TEAM_MEMBER_ROLE_MEMBER,
apps/api/src/app/services/team-seats.service.ts:467
- The row lock only covers the validation callback and is released before
applySeatChangecalls Stripe. Two admins can therefore load the sameexpectedCurrentSeats, both pass this check and the locked usage validation, and then concurrently update/charge or release the same subscription; the optimistic preview check does not prevent the race it promises to reject. Serialize the external mutation with a durable compare-and-set/version claim (and reconcile failures), or otherwise make concurrent commits retry safely.
await withTeamSeatLock(teamId, async (tx) => {
const { seats: usage } = await getTeamSeatSummary(tx, { teamId });
const validation = validateSeatRequest({ context, usage, requested: seats });
if (!validation.ok) {
throwSeatRejection(validation, { context, usage, minimumSeats: getMinimumSeats(context, usage) });
apps/cron-tasks/src/utils/backfill-team-seats.utils.ts:181
isAlreadySyncedomitspendingSeatQuantity,pendingSeatEffectiveAt, andseatScheduleId, despite the comment saying every mirrored field is compared. After a completed or cancelled schedule whose webhook was missed, the base fields can match while stale pending fields remain, so the backfill skips the team and the API continues to report/enforce a decrease that Stripe no longer has. Include these fields in the sync decision and clear stale pending values when Stripe has no schedule, while preserving them for a live schedule.
const isAlreadySynced =
account.licenseCountLimit === desiredSeats &&
account.seatQuantity === quantity &&
account.seatSubscriptionItemId === item.id &&
account.includedSeats === includedSeats &&
account.seatPeriodEnd?.getTime() === seatPeriodEnd.getTime();
libs/auth/server/src/lib/sso-auth.service.ts:500
- The invitation is read before the team-row lock, so it can be revoked or expire while this request waits. In that case
addMemberFromInvitationre-reads nothing and throws; unlike the new-user path above, an existing user with JIT enabled is not retried as an ordinary MEMBER JIT provision, so a valid SSO login fails instead of adding the user. Re-read the invitation under the lock and fall back to the JIT path when it is no longer available.
- Files reviewed: 86/87 changed files
- Comments generated: 3
- Review effort level: Lite
The preview had already drifted from the pricing page and the two would have to be kept in sync by hand. The pricing page is now the only place plans are described.
Team no longer requires a five-seat minimum: seats are billed per user, with a lower rate from the sixth seat. Plan copy moves into PRICING_COPY so the app billing page and the landing pricing page cannot drift, and Enterprise is presented as the custom-contract path rather than a tier. Existing customers stay on their current Stripe prices, and the billing page shows what they actually pay. That needs each tiered price's tier table, which Stripe omits from the price embedded in a subscription item and cannot be expanded through the customer because the path exceeds Stripe's depth limit, so tiers are fetched per price and cached.
|
Went through Copilot's suppressed (low-confidence) comments across all five reviews — a few were valid:
The rest were already handled in the current code or didn't apply (the past-due entry points are all disabled in place, the billing route is gated by the |
1720334 to
7a83294
Compare
7a83294 to
27950c6
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical invitation/SSO race conditions and moderate billing/state-handling issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (8)
apps/api/src/app/db/team.db.ts:1077
addMemberFromInvitationre-reads the invitation under the lock and may accept a different role than the one returned byverifyTeamInvitation, but this function discards that result and returnsexistingInvitation.role. If an admin re-roles the invitation between those reads, the success audit record reports the stale role even though the membership was created with the new one. Return the role from the locked acceptance result instead.
await withTeamSeatLock(teamId, (tx) => addMemberFromInvitation(tx, { teamId, userId: user.id, invitation: existingInvitation }));
apps/api/src/app/services/stripe-seats.service.ts:146
- If releasing the completed schedule fails for a transient Stripe error, this branch reports
foreignScheduleId: null. The sync then clears the pending mirror and seat-management requests proceed as if the subscription were free, but Stripe still has the schedule attached and rejects direct quantity updates. Preserve the schedule as foreign (or otherwise keep seat changes blocked) until release succeeds.
// The decrease landed when phase two started; release so the subscription is no longer schedule-managed
await releaseTeamSeatSchedule(schedule.id).catch((ex) => {
logger.warn({ scheduleId: schedule.id, ...getErrorMessageAndStackObj(ex) }, 'Unable to release applied seat decrease schedule');
});
return { pending: null, foreignScheduleId: null };
apps/api/src/app/services/team-seats.service.ts:340
- The Stripe quantity update has already succeeded before this invoice lookup, so an invoice-list failure makes the endpoint return an error even though the customer was charged and the seat increase was applied. That leaves the UI presenting a failed change and invites a retry; treat invoice retrieval as best-effort (log and return
invoice: null) after the mutation succeeds.
const invoice = await stripeSeatsService.fetchLatestInvoiceForSubscription(context.subscription.id);
apps/docs/docs/team-management/team-management.mdx:48
- The Seats card exposes
usedandreservedseparately, and availability is based on both. Describing the latter as part of “in use” makes the documented numbers inconsistent with the UI; a pending decrease also makes the effective limit lower than the purchased count.
The Seats card on the Team Dashboard shows how many seats you have purchased, how many are in use (including reserved seats for pending invitations), and how many are still available. If a decrease is scheduled, the card shows the new seat count and the date it takes effect. If more seats are in use than you have purchased, the card shows how many you are over.
apps/docs/docs/team-management/team-management.mdx:56
- A pending invitation reserves a seat, so the minimum is active billable members plus reserved invitations, not only the number currently in use. This wording can make a decrease rejected at the server minimum appear incorrect.
- You cannot reduce your seats below the number currently in use. Deactivate members or cancel pending invitations first.
apps/jetstream/src/app/components/billing/BillingExistingSubscriptions.tsx:114
availableis calculated againstseats.effective, which is the pending target during a scheduled decrease, but this text says the team is over its purchased count. It also tells manual-billing teams to buy seats even though self-service seat changes are blocked. Use the effective cap and provide support/deactivation guidance for manual billing.
Your team is using more seats than it has purchased ({seatsOver} {pluralizeFromNumber('seat', seatsOver)} over). Buy more seats
or deactivate members to avoid interruption.
libs/auth/server/src/lib/sso-auth.service.ts:123
rethrowSsoSeatLimitalways recordsattemptedAction: 'SSO_JIT', including failures fromaddMemberFromInvitationwhere the rejected operation is accepting an invitation. The resulting audit entry mislabels the membership path (and uses a team-member resource instead of the invitation-specific classification), so pass the actual action from the two SSO branches.
libs/features/teams/src/lib/TeamDashboard/team-seats/TeamSeatsManageModal.tsx:53- A self-serve team can have
seats.purchased === nullwhile the Stripe mirror is missing or has not run yet, but this fallback treats it as zero and initializes the modal atused + reservedseats. For an existing Stripe subscription this can make the first preview look like an increase in the UI while the server sees the real purchased count and schedules a decrease to that lower value. Keep Manage Seats unavailable until the purchased count is known (or otherwise load the live count) instead of convertingnullto0.
- Files reviewed: 95/96 changed files
- Comments generated: 3
- Review effort level: Lite
|
Went through Copilot's suppressed (low-confidence) comments across every review on this PR — several were valid and are now fixed:
Not changed:
Everything else suppressed had already been fixed in a later commit than the review that raised it. |
27950c6 to
0f6851f
Compare
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated 4 comments.
Suppressed comments (4)
libs/ui/src/lib/form/input/NumberStepperInput.tsx:1
- This component calls
setStateduring render (setSyncedValue/setInputValuein the function body). That can cause React warnings and render loops, and it breaks the rule that renders must be pure. Move this synchronization logic into auseEffectthat runs whenvaluechanges (and referenceinputValueas needed), or compute derived display state without mutating state in render.
libs/features/teams/src/lib/TeamDashboard/TeamMemberInviteModal.tsx:1 - The submit button both has
type=\"submit\"and anonClick={handleInvite}handler, while the formonSubmitcontains the actual validation (invalid email / seatBlocked). Clicking the button will invokehandleInviteviaonClickeven when theonSubmithandler would block, which can bypass validation in some cases. Remove theonClickand let the formonSubmitbe the single submission path, or change the button totype=\"button\"and centralize validation in the click handler.
libs/features/teams/src/lib/TeamDashboard/TeamMemberInviteModal.tsx:1 - The submit button both has
type=\"submit\"and anonClick={handleInvite}handler, while the formonSubmitcontains the actual validation (invalid email / seatBlocked). Clicking the button will invokehandleInviteviaonClickeven when theonSubmithandler would block, which can bypass validation in some cases. Remove theonClickand let the formonSubmitbe the single submission path, or change the button totype=\"button\"and centralize validation in the click handler.
libs/test/e2e-utils/src/lib/pageObjectModels/TeamDashboardPage.model.ts:1 NumberStepperInputclamps values on blur. As written, this helper will fail if any test callssetSeatCount()with a value that gets clamped (e.g., below min or above max). Consider either (a) clampingcountin this helper based on known bounds for the test, or (b) returning/expecting the post-clamp value so callers can assert accurately.
|
Went through Copilot's suppressed (low-confidence) comments from every review round — two were valid:
Everything else had already been addressed in later commits: tier attachment in Two I am not changing:
|
0f6851f to
8470937
Compare
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated 3 comments.
Suppressed comments (6)
libs/ui/src/lib/form/input/NumberStepperInput.tsx:1
- This component calls
setStateduring render (setSyncedValue/setInputValueinside theif (value !== syncedValue)block). That is an anti-pattern in React and can lead to render loops or warnings, especially under StrictMode. Move this synchronization logic into auseEffectthat runs whenvaluechanges (and consider whether you need bothsyncedValueandinputValuestate, or can derive one from the other).
libs/test/e2e-utils/src/lib/pageObjectModels/TeamDashboardPage.model.ts:1 - The seat footer copy in the product includes reserved-seat clauses in some states (e.g., over-allocated with
reserved > 0addsincluding X reserved for pending invitations). This e2e helper omitsreserved, so tests asserting on the footer can become incorrect/flaky for teams with pending invitations. Consider either (1) extendingSeatSummaryExpectationto includereservedand mirroring the real formatting rules, or (2) reusing the same formatter used by the UI (getSeatFooterMessage) so the contract is literally shared.
libs/test/e2e-utils/src/lib/pageObjectModels/TeamDashboardPage.model.ts:1 - The seat footer copy in the product includes reserved-seat clauses in some states (e.g., over-allocated with
reserved > 0addsincluding X reserved for pending invitations). This e2e helper omitsreserved, so tests asserting on the footer can become incorrect/flaky for teams with pending invitations. Consider either (1) extendingSeatSummaryExpectationto includereservedand mirroring the real formatting rules, or (2) reusing the same formatter used by the UI (getSeatFooterMessage) so the contract is literally shared.
libs/team-seats/src/lib/seat-enforcement.ts:1 getTeamSeatSummaryperforms the team lookup, then runs the two independent count queries sequentially. SinceusedSeatsandreservedSeatsdon’t depend on each other, usingPromise.allfor the counts (and possibly running the lookup in parallel if appropriate) would reduce latency on hot membership-change paths.
libs/team-seats/src/lib/seat-enforcement.ts:1getTeamSeatSummaryperforms the team lookup, then runs the two independent count queries sequentially. SinceusedSeatsandreservedSeatsdon’t depend on each other, usingPromise.allfor the counts (and possibly running the lookup in parallel if appropriate) would reduce latency on hot membership-change paths.
libs/team-seats/src/lib/seat-enforcement.ts:1getTeamSeatSummaryperforms the team lookup, then runs the two independent count queries sequentially. SinceusedSeatsandreservedSeatsdon’t depend on each other, usingPromise.allfor the counts (and possibly running the lookup in parallel if appropriate) would reduce latency on hot membership-change paths.
8470937 to
af6e8cb
Compare
|
Went through Copilot's suppressed (low-confidence) comments from this round — one was valid:
The other two did not apply:
|
There was a problem hiding this comment.
🟡 Changes recommended
Critical billing-state synchronization and Stripe reconciliation issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
libs/team-seats/src/lib/seat-audit.ts:43
SeatLimitErrorincludes bothNO_SEATSandPAST_DUE, but every rejection is recorded asTEAM_MEMBER_ADD_BLOCKED_NO_SEATS. A membership change denied because the team's billing is past due is therefore displayed in the audit log as a capacity denial, which is misleading even though the raw metadata contains the code. Use a neutral action or select a distinct action/label based onerror.code.
- Files reviewed: 97/98 changed files
- Comments generated: 3
- Review effort level: Lite
| @@ -155,29 +322,30 @@ export const updateTeamSubscriptionStateForCustomer = async ({ | |||
| */ | |||
| const hasSubscriptions = subscriptions.length > 0; | |||
| const isPastDue = subscriptions.some(({ status }) => status === 'past_due'); | |||
| const activeSubscriptions = filterInactiveSubscriptions(subscriptions); | ||
| await subscriptionDbService.updateTeamSubscriptionStateForCustomer({ | ||
| teamId, | ||
| customerId: customer.id, | ||
| subscriptions: filterInactiveSubscriptions(subscriptions), | ||
| subscriptions: activeSubscriptions, | ||
| seatState: await resolveTeamSeatState(activeSubscriptions), | ||
| }); |
| // A partial change (e.g. schedule released, then payment declined) must still be reflected locally | ||
| await resynchronizeSeatState(context).catch((syncError) => { | ||
| logger.warn({ teamId, ...getErrorMessageAndStackObj(syncError) }, 'Unable to re-synchronize seat state after a failed seat change'); | ||
| }); | ||
| throw ex; |
Teams now buy a specific number of seats instead of being capped at a flat member limit. The purchased count is mirrored from Stripe into team_billing_account and enforced on every path that can add a billable member: invitations, invitation acceptance, reactivation, role changes and SSO/team join. Admins and billing users manage seats from the team dashboard. Increases are prorated and charged immediately; decreases are scheduled with a Stripe subscription schedule and take effect at the end of the billing period, and can be cancelled by returning to the purchased count. Billing-only members never consume a seat. Pending invitations reserve one until they expire. Manual-billing teams keep a hand-set cap that sync never touches. Adds a one-off backfill entry point (backfill-team-seats) that mirrors each existing self-serve team's Stripe quantity into the new columns.
af6e8cb to
b57755d
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate and critical findings remain before approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
apps/api/src/app/services/team-seats.service.ts:470
- The live Stripe seat state is loaded and compared before acquiring the team lock. Two admins can therefore commit previews with the same
expectedCurrentSeatsconcurrently; both pass this check, both mutate Stripe (and increases can create two proration invoices), and the last request silently wins. Re-check or reserve a version under the lock, and serialize the Stripe mutation so a second commit is rejected as stale.
const context = await loadSeatContext(teamId);
if (expectedCurrentSeats !== context.effectiveSeats) {
libs/auth/server/src/lib/sso-auth.service.ts:492
- The same race exists in the existing-user path: the
!invitation && !allowJitcheck above this transaction can reject before the locked re-read when an invitation is created concurrently. Keep the invitation/JIT decision inside this callback, where the current team state is protected by the lock, instead of allowing the stale pre-lock snapshot to decide the outcome.
libs/features/teams/src/lib/TeamDashboard/TeamMemberUpdateModal.tsx:27 - This condition does not account for membership status. Changing an inactive Admin/Member to a billing-only role does not free a seat because inactive billable members are not counted in
used, but the modal still shows the "frees a seat" notice. Gate this notification onteamMember.status === TEAM_MEMBER_STATUS_ACTIVE.
- Files reviewed: 97/98 changed files
- Comments generated: 2
- Review effort level: Lite
| const [firstTier] = tiers ?? []; | ||
| // Explicit null check rather than a falsy one: a first tier priced at $0 covers its seats for free, | ||
| // and reading that as "no flat amount" would shrink the team's cap to the raw item quantity | ||
| if (!firstTier || firstTier.flatAmount === null || firstTier.upTo === null) { |
| const newUser = await withTeamSeatLock(teamId, async (tx) => { | ||
| // The invitation was read before the lock, so read the current one here rather than reusing that | ||
| // snapshot: one revoked or re-roled in the meantime must not still hand out its role, features or | ||
| // seat reservation, and one created while we waited must be applied instead of left pending — | ||
| // otherwise it reserves a seat forever behind a membership that ignored it. | ||
| const currentInvitation = await tx.teamMemberInvitation.findFirst({ |
Moves the Team plan from a flat 20-member cap to purchased seats, priced per user with volume tiers.
What changed
Seat model. Teams buy a specific number of seats. The purchased count is mirrored from Stripe into
team_billing_accountand enforced on every path that can add a billable member: creating an invitation, resending one, accepting one, reactivating a member, changing a role, and joining through SSO. Billing-only members never consume a seat. Pending invitations reserve one until they expire. Manual-billing teams keep a hand-set cap that sync never touches.Managing seats. Admins and billing users get a Manage Seats flow on the team dashboard that previews the charge before committing. Increases are prorated and charged immediately. Decreases are scheduled with a Stripe subscription schedule, take effect at the end of the billing period, and are cancelled by returning to the purchased count. A blocked membership change is written to the team audit log so admins can see who was turned away.
Pricing. Team is now per user with a volume break at 6 seats. Monthly is $30/user, dropping to $25. Annual is $25/user, dropping to $21. Display copy is shared between the app and the landing page through one constant so the two cannot drift.
Backfill. A one-off
backfill-team-seatsentry point mirrors each existing self-serve team's Stripe quantity into the new columns. It supportsDRY_RUN=true, skips teams whose Stripe state is ambiguous, and exits non-zero if any team could not be read.Also on this branch
Three earlier commits change public-facing content that is not part of the seat work: the SOC 2 Type II badge and report availability on the landing site, the removal of the pricing preview from the home page, and the related privacy-policy and DPA updates. They are here because the pricing page had to change alongside the new Team pricing. Review them on their own terms.
Deploying
Requires a schema migration, new Stripe prices carrying the
TEAM_MONTHLYandTEAM_ANNUALlookup keys with volume tiers, fivesubscription_schedule.*webhook events enabled, andSTRIPE_API_KEYavailable to the cron app for the backfill. Full runbook is in the artifact shared alongside this PR.Testing
Nine new end-to-end scenarios cover the cap, reservations, role changes, pending decreases, over-allocation, expired invitations, manual billing, and the preview/commit round trip. Unit coverage was added for the seat math, enforcement, the seat service, the backfill, and the stepper input.