ποΈ PUT-1712 + PUT-1747: emit billing events and report held bytes - #3722
Closed
Conversation
Contributor
Coverage Report
File Coverage |
jfcastro92
force-pushed
the
juancastro/put-1712-33-emit-on-demand-billing-events-for-the-payment-integration
branch
from
September 2, 2026 19:32
e53d8c9 to
74fbe54
Compare
jfcastro92
force-pushed
the
juancastro/put-1712-33-emit-on-demand-billing-events-for-the-payment-integration
branch
from
September 2, 2026 19:39
74fbe54 to
2c23865
Compare
jfcastro92
force-pushed
the
juancastro/put-1712-33-emit-on-demand-billing-events-for-the-payment-integration
branch
from
September 2, 2026 20:05
2c23865 to
e2373e4
Compare
jfcastro92
force-pushed
the
juancastro/put-1712-33-emit-on-demand-billing-events-for-the-payment-integration
branch
from
September 2, 2026 21:38
e2373e4 to
3668feb
Compare
Salazareo
requested changes
Sep 3, 2026
Salazareo
left a comment
Member
There was a problem hiding this comment.
might need to talk this over
jfcastro92
force-pushed
the
juancastro/put-1712-33-emit-on-demand-billing-events-for-the-payment-integration
branch
from
September 3, 2026 13:47
3668feb to
62ad1dc
Compare
jfcastro92
force-pushed
the
juancastro/put-1712-33-emit-on-demand-billing-events-for-the-payment-integration
branch
from
September 3, 2026 14:30
62ad1dc to
63f96fb
Compare
Schema foundations for teams. Ships dark β nothing reads these columns yet. group: kind, name, handle, plan_id, deleted_at jct_user_group: org_owned user: requires_password_change `kind = 'team'` marks a workspace; the seeded system groups keep it NULL so a team query can never surface them. `org_owned` distinguishes accounts the workspace created from the master account, deciding who pays rather than who may read. `requires_password_change` is a fourth `requires_*` flag for assertVerifiedAccount, needed by the phase 5 reset flow but shipped here to avoid a second three-dialect migration. `handle` uniqueness is case-insensitive, and the dialects disagree by default: mysql gets it from utf8mb4_unicode_ci, sqlite needs COLLATE NOCASE (as 0055 does for usernames), and postgres indexes lower(handle). Without this the same migration would accept `Design-Team` next to `design-team` on sqlite and postgres while mysql rejected it. Postgres handle lookups must therefore compare lower(handle) to use the index. idx_group_owner is sqlite-only: mysql and postgres already index that column. idx_jct_user_group_group is composite, unlike the existing single-column keys.
Review follow-ups on the team columns. NOCASE moves onto the `handle` column itself, not just the index. Index-only NOCASE makes uniqueness case-insensitive while leaving `WHERE handle = ?` case-sensitive, so the same lookup would match on mysql (utf8mb4_unicode_ci) and miss on sqlite. Postgres still needs lower(handle) at the call site. `requires_password_change` becomes NOT NULL DEFAULT 0 on all three dialects, matching the three sibling `requires_*` flags. Left nullable, any query written as `= 0` would silently exclude every pre-existing user.
\`jct_user_group\` had no unique constraint on (user_id, group_id) and \`GroupStore.addUsers\` had no conflict clause, so re-adding a member inserted a second row. \`readUserGroupPerms\` joins the junction table on group_id alone, so each duplicate returned another copy of every group permission the user holds. Deduplicate keeping the lowest id, add the unique pair index, and make \`addUsers\` ignore conflicts via the existing \`insertIgnoreInto\` helpers -- without that last part the index turns a re-add into a raised error, which five call sites would log as a failed signup step. mysql cannot delete from a table it reads in a subquery (error 1093), so it uses a self-join with the same lowest-id-wins semantics.
Review follow-ups on the dedup migration. mysql and postgres track no per-file applied state and re-execute every migration on each boot, so the unguarded DELETE self-joined the whole table at every process start, forever. Both now sit behind the same index-existence check that guards the ALTER, which also stops a rolling deploy deleting on one instance while another adds the index. The dedup test was a false green. `targetVersion: 67` never applies 0071 -- the loop breaks on `threshold + 1 >= targetVersion` but stamps the target anyway -- so the fixture asserted the current schema version on a database missing a migration. It now replays the real 0072 file against a fully migrated database, and a second test pins the off-by-one so nobody builds a fixture on it again.
Insert-only record of what a workspace administrator did to an account, shaped like \`audit_user_to_group_permissions\` after 0019: nullable FK beside a NOT NULL \`_keep\` column. The FKs are ON DELETE SET NULL, never CASCADE, so hard-deleting an account cannot erase the record of the resets performed on it. Two indexes rather than one. The member's own view is the only place a reset becomes visible to the account it was performed on, so (user_id_keep, id) is a read path, not an optimisation. \`share.holder_group_id\` mirrors \`holder_user_id\` from 0067. The existing unique index does not constrain team shares at all -- it leads with \`holder_user_id\`, which is NULL on every team share, and NULLs are distinct -- so the group-scoped unique index is what prevents duplicates. Drops the \`role\` column from the specified DDL: it contradicted the settled single-administrator model.
Review follow-ups on the audit table. The three FK columns are ON DELETE SET NULL, so every user or group delete has to find its child rows. sqlite and postgres had no index on them and would scan the whole append-only audit table -- twice for a user, since actor_user_id points at `user` as well. mysql already had them via its FK KEY declarations; those are renamed to the same idx_*_fk scheme so the three dialects match. Also fixes AND/OR precedence in the index-list assertion, which left the second LIKE unscoped to indexes and passed by luck.
`GroupStore` has only addUsers/removeUsers; nothing creates, reads back or lists a group at runtime. `TeamStore` is that missing half, scoped to rows with `kind = 'team'`. A workspace is addressed by `uid`, which `group` has carried as NOT NULL UNIQUE since 0015. `handle` is a mutable display label with no addressing role, so a rename invalidates nothing and a stale reference can never resolve to a different workspace. Soft delete releases the handle and keeps `name`. Nothing points at a handle, so the name returns to the pool instead of being reserved forever by a global unique index that cannot exclude dead rows -- mysql has no partial indexes, so that exclusion was never available. Handles validate to ^[a-z0-9]+(-[a-z0-9]+)*$, 3-64 chars, against a reserved list. The charset is deliberately narrower than the column so the engines' collations cannot disagree: mysql's utf8mb4_unicode_ci also folds accents and eszett, which sqlite's NOCASE and postgres's lower() do not. Every read filters `kind = 'team' AND deleted_at IS NULL`, which is what makes the seeded admin/system groups unreachable rather than merely absent. Handle lookups compare lower(handle) on postgres, where the index is on that expression rather than the column.
Membership management for workspaces: addMember, removeMember, getMembership, isMember, listMembers and listTeamsForUser. The permission scan is untouched -- readUserGroupPerms already joins jct_user_group and resolves group grants; this is the management side. Resolves the ticket's "do not leave two writers" by splitting domains and enforcing the split in SQL rather than by convention. Every existing caller of GroupStore targets a seeded system group -- ADMIN_GROUP_UID, default_user_group, default_temp_group -- never a team, so the two stores were already disjoint in practice. GroupStore.addUsers/removeUsers now carry `AND kind IS NULL`, making a team uid a no-op there, which costs no extra query because it folds into the existing subquery and matches how addUsers already treats an unknown username. TeamStore's writes select group_id from a kind-filtered subquery, so neither store can reach the other's rows. org_owned is written here but never accepted from a request; TeamService sets it at provisioning and workspace creation only. listMembers is keyset-paginated on id per doc/pagination.md, using the shared cursor and limit helpers and fetching one row past the limit to decide whether a cursor is warranted. Passes 1/0 for org_owned rather than db.booleanValue, which yields a real boolean on postgres and is rejected by the smallint column there -- sqlite accepted it silently.
Covers PUT-1704 and PUT-1707: creating a workspace, admitting the master account, and the whole of offboarding. `createWorkspace` admits the creator with org_owned = 0, which is what makes the master pay for itself and stay an invalid target of every member route. `checkOwnerInvariant` asserts the rule no dialect can express -- the owner is a member with org_owned = 0 and the only such member -- and a test breaks it deliberately, since the schema cannot refuse a second one. Three authority checks: 404 to a stranger so the endpoint is not an existence oracle, 403 to a member who is not the master, and the master refused as a target of member routes. Handle problems surface as 400 (unusable) or 409 (taken), including the unique-index race. `TeamStore` throws a bare Error, which the server would turn into a 500 and a deduped critical alarm -- an uppercase handle should not page on-call. Disable writes `user.suspended` as well as suspended_at and suspended_reason. PUT-1707 named only the latter two, but those are siblings added by 0061 and 0063 -- `userProtected` rejects on `if (user.suspended)` and reads neither. Setting only the timestamp and reason would have recorded a disable that never took effect, and disable is the whole of offboarding here. Sessions are dropped through SessionStore.removeByUuid rather than a raw DELETE. The store invalidates every composite cache key with its double-delete pattern; without that a disabled member keeps authenticating from cache for the session TTL, which is exactly the "next request, not after a cache TTL" property disable is supposed to have. Revoking also preserves last_ip and last_user_agent, which the member-facing audit view reads. Files are untouched and re-enable restores the account. Adds team_not_found, not_the_master_account and not_an_org_account to the HttpError legacy codes, which the controller also needs. Billing events, invalidateActorSubscription, audit rows and the GUI push are deliberately not here -- they belong to phase 3 and PUT-1708.
Covers PUT-1705. The master account supplies { username, email }; the account
is created with no password, gets the default filesystem tree, joins with
org_owned = 1, and receives a one-shot activation link.
Activation reuses password recovery rather than new token machinery: the same
pass_recovery_token, the same one-hour purpose-scoped JWT, the same
/action/set-new-password link. No team_activation table, no new token type,
and no unauthenticated endpoint on the team surface. Activation state needs no
column either -- an unactivated account is one with no password.
Applies the same username and email rules as signup rather than its own:
USERNAME_REGEX, USERNAME_MAX_LENGTH, RESERVED_USERNAMES and validator.isEmail,
now exported from AuthController. Without them a workspace could mint accounts
signup would refuse -- the username becomes the /username home-directory
segment -- claim unregistered reserved names, and send activation mail to
arbitrary unvalidated addresses at the route's daily limit.
Usernames come from Puter's global pool, so a taken one is refused with free
alternatives rather than silently modified: a suffixed name would appear in
every share dialog that person ever sees, and they never agreed to it. The
check runs before any write, so a rejected provision leaves no orphaned user
row -- asserted by a test on the workspace's member count.
The new account carries requires_email_confirmation, since the address came
from the administrator rather than its holder.
Adds a team_account_activation email template stating what the workspace can
and cannot do -- including that it can reset the password, which the design
requires be said rather than only claiming files are private.
free_storage stamping and the billing event are phase 3.
β¦uite Covers PUT-1708, PUT-1709 and PUT-1743. Twelve routes, every one setting requireUserActor -- that option is what installs requireAuthGate, requireVerifiedAccount and requireNonAccessTokenGate, because server.ts derives `needsAuth` from the route options. Reads need it as much as writes: without an auth option a route gets no suspension check and admits access tokens, so a just-disabled member could still read the roster and a scoped third-party token could read the audit log. Authority is checked before anything observable. Validating the body first made POST /members answer 400 before 403, and resolving :username first turned the member routes into a global username-existence oracle. Provisioning applies the same username and email rules as signup rather than its own -- USERNAME_REGEX, USERNAME_MAX_LENGTH, RESERVED_USERNAMES and validator.isEmail, now exported from AuthController. Without them a workspace could mint accounts signup would refuse, claim unregistered reserved names, and mail arbitrary unvalidated addresses. Handle problems are 400 or 409 rather than a bare Error, which the server turns into a 500 and a deduped critical alarm -- an uppercase handle should not page on-call. Disable drops sessions through SessionStore.removeByUuid rather than a raw DELETE. The store invalidates every composite cache key; without that a disabled member kept authenticating from cache for the session TTL, which is exactly the "takes effect on the next request, not after a cache TTL" property disable is supposed to have. Revoking also preserves last_ip/last_user_agent, which the member-facing audit view reads. Audit writes live in TeamService at the point of each action rather than in the route, so a caller reaching the service directly cannot skip them, and the SQL lives in TeamStore. Audit reads map internal user ids to usernames, and remain readable by the owner after the workspace is soft-deleted -- otherwise the delete_team entry was written and immediately unreachable. teams_enabled gates route registration through an optional isEnabled() the server honours, so with it off the paths do not exist rather than existing and refusing. It does not gate DDL. TeamIsolation.http.test.ts asserts the negative the feature rests on: the workspace manages accounts and cannot read them, including through a full-access token and after the member is disabled. It asserts outcomes rather than the absence of an implicator.
`addMember` refuses to turn an account that already has a password into a workspace seat. No service path did this β `provisionAccount` always creates β but the store permitted it, and the design rules out existing accounts joining a workspace. Provisioning passes the guard because it admits the account before setting its temporary password. There is no bypass parameter. Both HTTP suites now provision a real seat and authenticate as it, using the same token-minting the harness uses for `POST /login`. That surfaced something worth knowing: an unactivated seat cannot call the API at all. Provisioning leaves `requires_email_confirmation` set and `requireVerified` rejects it, so the suites activate the seat first β which is the state a member is actually in when making requests. `listMembers` and `getMembership` also return `u.uuid`, which the billing events need in order to name the account without a second lookup.
The charge lives outside this repo, the way the marketplace extension cancels Stripe subscriptions off `user.delete`. This is the trigger, and it is the whole of what OSS owes billing. team.account.created a seat exists and can be used team.account.disabled it stopped, and still holds its bytes team.account.enabled it resumed team.account.deleted it is gone team.deleted the workspace is gone; its accounts are not Each carries the workspace uid, the affected account, and the owner's `stripe_customer_id`. That column ships in the mysql and postgres schemas but not sqlite, so the read is guarded and degrades to null, as `cascadeDelete` already does. Deleting a workspace emits one `team.account.disabled` per seat plus the workspace event, rather than one bulk event: the accounts persist, suspended, holding their files and their usernames. Deleting a workspace is not a way to stop paying for the accounts in it. `team.account.deleted` is captured before the row is deleted. `jct_user_group.user_id` is ON DELETE CASCADE, so by the time a listener on `user.delete` runs, nothing can say which workspace paid for the account. `getOrgSeat` deliberately admits soft-deleted workspaces: their accounts still exist, so the charge is still running. Closes PUT-1712.
jfcastro92
force-pushed
the
juancastro/put-1712-33-emit-on-demand-billing-events-for-the-payment-integration
branch
from
September 3, 2026 15:42
63f96fb to
86d5fb1
Compare
Collaborator
Author
|
Reopening on the correct base β #3721 closed, so this was stacked on a dead branch. Continued in the PR that supersedes it. Content is also rescoped: the held-bytes report is deleted rather than moved (see TEAMS-BILLING-SPLIT.md). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Covers PUT-1712 (on-demand billing events) and PUT-1747 (the recurring held-bytes report). Grouped because they are the two halves of one signal: what a workspace owes at each transition, and what it owes in between.
Nothing here computes money. The price lives in
TEAM_MEMBER_POLICYand the charge lives outside this repo β the same place that cancels Stripe subscriptions offuser.delete. That event is the precedent for the shape, and this follows it: emit a typed event, let an extension do the external side effect.The events
Each carries the master account's
stripe_customer_id(it is the payer), the workspace uid, and the seat.team.account.createdteam.account.disabledteam.account.enabledteam.account.deletedteam.deletedteam.held-bytes.reportDisable and re-enable carry
held_bytesat the moment of transition, so the payment side has a figure without querying back.Deleting a workspace emits per seat, not in bulk
The ticket describes workspace deletion as one row. It is emitted as one
team.account.disabledper seat plus ateam.deletedmarker, because the byte charge that follows is per account β a bulk event would carry an unbounded account list and still need unpacking into per-account charges on the other side.deleteWorkspacealready disables each seat, so the per-seat events are what actually happened.β Worth stating plainly, since PUT-1740's confirmation dialog depends on it: deleting a workspace is not a way to stop paying. The seats stop, the bytes do not β the accounts still exist and still hold data. Deleting the accounts is what stops the byte charge.
β
team.account.deletedhas to be captured before the deletejct_user_group.user_idisON DELETE CASCADE(0015_group.sql). By the time a listener onuser.deleteruns, the membership row is gone and nothing can say which workspace paid for the account β the payload is the only surviving evidence.So
cascadeDeletecaptures the seat identity before teardown and emits after, exactly as it already does foruuid/stripe_customer_idand for the same reason. There is a test asserting the membership is unreadable afterwards, so the constraint that forces this design is pinned rather than assumed.β
stripe_customer_iddoes not exist on sqliteIt ships in
mysql_mig_1andpostgres_mig_1but no sqlite migration βcascadeDeletealready wraps its read in atry/catchfor this.getStripeCustomerIddoes the same and degrades tonull, so a dev or self-hosted install on sqlite emits usable events rather than throwing inside a lifecycle operation. Production is mysql/postgres, where the value is real; the postgres suite covers the branch that actually returns one.The recurring report
TeamBillingServiceβ separate fromTeamServicebecause it is scheduled rather than request-driven, the same split asShareNotificationServicefromShareService.fs.write.file. A disabled account cannot write, so its total only changes when the account is deleted, which emits its own event.observed_atis the window start, floored to the interval β not the sample instant. Billing invoices a window; an arbitrary timestamp is not one, and flooring is also what makes two runs byte-identical.team:held-bytes:lock:<periodStart>), so a second node in the same window is refused rather than racing. Each node's timer has its own phase from its own boot, so a duration-based TTL would not have deduplicated them.SUM(size)is the source, as the ticket specifies.org_owned = 1only.Cadence is
team_held_bytes_report_hours, default 24. The sweep is bounded at 50 000 accounts per run and logs when it truncates β a short report reads as a smaller bill, so it must never be silent.The
org_ownedfilter is load-bearingA master account that is itself suspended by Puter would otherwise appear in its own workspace's held-bytes report, moving its personal storage onto the workspace's bill. The master pays for its own storage under its own plan. There is a test for exactly that shape β a suspended master with ~1 MB against a disabled seat with 64 bytes, asserting only the seat is reported.
I did not write that test until falsification told me to; see below.
Verification
Postgres matters here beyond the usual: it is the engine where
stripe_customer_idexists and wheresuspendedis a realbooleanrather thantinyint(1), so the report's filter is built withdb.booleanValue(true)rather than a literal1.Per-site falsification β every emit and filter, one at a time
Each site disabled individually, with the suite re-run each time:
Every site fails exactly one test: no site is untested, and no test is over-broad.
Two of these only became true because the first pass was wrong, which is the argument for doing it per-site:
org_ownedfilter both survived their mutation. Flooring was only incidentally covered by the two-runs-are-identical test, which catches it just by timing; and nothing at all coveredorg_owned, because no test had a suspended master. Both now have a dedicated test.Not here
monthlyStorageAllowanceremains declarative βuser.free_storageis whatFSServiceenforces, as noted in #3721.Closes PUT-1712 and PUT-1747.