Skip to content

🏗️ PUT-1703: TeamStore — membership reads and writes (single writer) - #3710

Merged
jfcastro92 merged 1 commit into
juancastro/put-1702-21-teamstore-workspace-crud-handle-uniqueness-reserved-wordsfrom
juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer
Sep 3, 2026
Merged

🏗️ PUT-1703: TeamStore — membership reads and writes (single writer)#3710
jfcastro92 merged 1 commit into
juancastro/put-1702-21-teamstore-workspace-crud-handle-uniqueness-reserved-wordsfrom
juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer

Conversation

@jfcastro92

@jfcastro92 jfcastro92 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fifth in the stack: #3704#3705#3708#3709 → this. Review bottom-up; the diff here is the membership half of TeamStore.

The management side of workspace membership. readUserGroupPerms already joins jct_user_group and resolves group grants for the permission scan — that path is untouched.

getMembership(teamUid, userId)      addMember(teamUid, userId, { orgOwned })
isMember(teamUid, userId)           removeMember(teamUid, userId)
listMembers(teamUid, { limit, cursor })
listTeamsForUser(userId)

The "single writer" decision

The ticket asked to decide whether TeamStore supersedes GroupStore.addUsers/removeUsers, and not to leave two writers.

First, what the callers actually do. All five production call sites target a seeded system group:

Call site Group
AuthController signup ×2, save-account ×1 default_user_group, default_temp_group
OIDCService first login default_user_group
DefaultUserService bootstrap ADMIN_GROUP_UID

Never a team — those uids come from config and constants. So the two stores were already disjoint in practice, and the ticket's stated fear ("the one without the conflict clause will eventually be the one that is called") was defused by #3705 making addUsers conflict-tolerant.

So rather than delete GroupStore and churn the signup and OIDC paths inside a teams PR, the split is now enforced in SQL:

  • TeamStore writes select group_id from a kind = 'team' subquery.
  • GroupStore.addUsers/removeUsers carry AND kind IS NULL.

Neither store can reach the other's rows. That costs zero extra queries — it folds into subqueries both methods already had — and a team uid becomes a no-op, which is how addUsers already treats an unknown username (there's a pre-existing test for that). Two new tests on the GroupStore side assert it cannot touch a team.

⚠ If you'd rather have one literal writer and accept the blast radius, that's a GroupStore deletion plus five call-site rewrites through signup, OIDC and the self-hosted bootstrap. I judged that too invasive to smuggle into this PR, but say so and I'll do it separately.

A cross-dialect bug worth seeing

addMember first used db.booleanValue(orgOwned). That looked right and passed on sqlite — 27/27 green.

PostgresDatabaseClient overrides booleanValue to return a real boolean, because postgres has a real boolean type. But org_owned is an integer flag in all three dialects (tinyint(1) / INTEGER / smallint), so postgres rejected it outright:

error: invalid input syntax for type smallint: "true"
  Tests  10 failed | 26 passed (36)

SQLite is loosely typed and stored true without complaint. Now passing 1/0 explicitly, with a comment saying why so nobody "tidies" it back. This only surfaced because both engines were run.

Other details

org_owned is written here but never accepted from a requestTeamService sets it at provisioning and workspace creation. It decides who pays, not who may read, and a test asserts the workspace owner's row carries 0 while a provisioned account carries 1.

listMembers is keyset-paginated on id per doc/pagination.md, using the shared encodeCursor / decodeCursor / normalizeLimit helpers and fetching one row past the limit to decide whether a cursor is warranted.

Reads inherit the kind = 'team' AND deleted_at IS NULL predicate, so soft-deleting a workspace drops it out of listTeamsForUser and isMember without touching the membership rows.


Verification

Both engines

$ npx vitest run --config src/backend/vitest.config.ts src/backend/stores/team/ src/backend/stores/group/
 Test Files  2 passed (2)
      Tests  36 passed (36)

$ PUTER_TEST_DB_ENGINE=postgres npx vitest run --config src/backend/vitest.config.ts src/backend/stores/team/ src/backend/stores/group/
 Test Files  2 passed (2)
      Tests  36 passed (36)

Eleven tests added — membership round-trip, org_owned distinguishing the workspace owner, repeat-add being a no-op, removal reporting whether a row existed, workspace scoping, soft-delete dropping out of the user's list, addMember refusing a non-team group, and two on the GroupStore side asserting the boundary.

Two pagination tests: a 5-member workspace walked in pages of 2 terminates in exactly 3 pages with every member seen once, and limit: 100_000 is capped at 200 rather than trusted.

Every write to `jct_user_group` in the codebase, after this change
GroupStore.ts:61   DELETE FROM `jct_user_group`        <- system groups (kind IS NULL)
GroupStore.ts:46   INSERT ... INTO `jct_user_group`    <- system groups (kind IS NULL)
TeamStore.ts:349   DELETE FROM `jct_user_group`        <- teams (kind = 'team')
TeamStore.ts:330   INSERT ... INTO `jct_user_group`    <- teams (kind = 'team')

Two stores, disjoint by predicate, one writer per domain.

Full suite and typecheck

$ npm run test:backend
 Test Files  251 passed | 24 skipped (275)
      Tests  6730 passed | 26 skipped (6756)      # +12, no regressions

$ npm run typecheck
Type check passed — no new errors (33 known, baselined).

Closes PUT-1703.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 93.97%
🟰 ±0%
27403 / 29160
🔵 Statements 92.08%
🟰 ±0%
29705 / 32259
🔵 Functions 90.38%
⬇️ -0.01%
4926 / 5450
🔵 Branches 80.93%
⬆️ +0.02%
19766 / 24422
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/backend/stores/group/GroupStore.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
src/backend/stores/team/TeamStore.ts 93.9% 90.19% 88.88% 94.73% 181, 219, 245, 362-372
Generated in workflow #1444 for commit 8b4e259 by the Vitest Coverage Report Action

@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 5629ae7 to 7d8050a Compare September 1, 2026 20:09
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 7d8050a to 8275ec2 Compare September 1, 2026 21:36
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 8275ec2 to 7b1a1ba Compare September 1, 2026 22:04
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 7b1a1ba to 1e86d0e Compare September 1, 2026 22:50
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 1e86d0e to 1ac35af Compare September 2, 2026 16:05
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 1ac35af to ed582d0 Compare September 2, 2026 16:51
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from ed582d0 to 00f6d00 Compare September 2, 2026 19:32
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 00f6d00 to c24b424 Compare September 2, 2026 19:39
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from c24b424 to 0cbd86b Compare September 2, 2026 21:37
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 0cbd86b to 5573c92 Compare September 3, 2026 13:46
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 5573c92 to 21b8b8d Compare September 3, 2026 14:30
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 21b8b8d to e97dbda Compare September 3, 2026 15:42
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from e97dbda to eeb4d4c Compare September 3, 2026 18:56
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from eeb4d4c to c5b7c99 Compare September 3, 2026 19:07
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from c5b7c99 to d82c8a7 Compare September 3, 2026 19:17
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from d82c8a7 to 3e84e87 Compare September 3, 2026 19:20
@jfcastro92

Copy link
Copy Markdown
Collaborator Author

Local validation

Engine: mysql 8 · workspace with 4 members (1 owner + 3 provisioned seats)

Full listing

GET /teams/<uid>/members -> 200
{"items":[{"username":"tmowner","org_owned":false,"created_at":"2026-09-03T23:25:43.000Z"}]}

…before seats existed, and after provisioning, paginated below.

Keyset pagination, page 1 (?limit=2)

{"items":[{"username":"tmowner",  "org_owned":false,"created_at":"2026-09-03T23:25:43.000Z"},
          {"username":"acmeseat1","org_owned":true, "created_at":"2026-09-03T23:27:28.000Z"}],
 "cursor":"eyJpZCI6ODh9"}

Page 2 (?limit=2&cursor=eyJpZCI6ODh9)

{"items":[{"username":"acmeseat2","org_owned":true,"created_at":"2026-09-03T23:27:29.000Z"},
          {"username":"acmeseat3","org_owned":true,"created_at":"2026-09-03T23:27:29.000Z"}]}

Three properties worth calling out, all matching doc/pagination.md:

  • No overlap — page1 ['tmowner','acmeseat1'], page2 ['acmeseat2','acmeseat3'], intersection empty.
  • The last page omits cursor entirely rather than returning a null or an empty
    string, so "is there more" is a key-presence test.
  • Ordering is stable across the page boundary even though acmeseat2 and
    acmeseat3 share a created_at to the second — the cursor is keyed on id
    (eyJpZCI6ODh9 decodes to {"id":88}), not on the timestamp, so the tie does not
    cause a repeat or a skip. That is the case a same-second insert would break under
    timestamp keying, and it is exercised here by real rows rather than by fixture.

org_owned is correctly false for the owner and true for provisioned seats,
returned as a JSON boolean rather than mysql's 1/0.

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.
@jfcastro92
jfcastro92 force-pushed the juancastro/put-1703-22-teamstore-membership-reads-and-writes-single-writer branch from 3e84e87 to 8b4e259 Compare September 3, 2026 20:02
@jfcastro92
jfcastro92 merged commit d855315 into main Sep 3, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants