🏗️ PUT-1701: Add audit_team_membership and share.holder_group_id

[jfcastro92]

Stacked on #3705 (PUT-1700), which is stacked on #3704 (PUT-1699). Review those first — this branch is cut from #3705, so the diff here is only the third migration.

Last schema ticket of phase 1. Adds the one new table the feature needs, plus the share column that lets a workspace-held share list like a user-held one. Ships dark — nothing reads either yet.

audit_team_membership   new table, insert-only
share:                  holder_group_id

audit_team_membership

Shape follows audit_user_to_group_permissions as fixed in 0019: a nullable FK beside a NOT NULL _keep column, so a row still names its subject after the FK is blanked. (0016 had the nullability inverted; 0019 is the one to copy.)

Two properties are load-bearing:

• ON DELETE SET NULL, never CASCADE. With cascade, hard-deleting an account would erase the record of the password resets performed on it — the exact evidence the table exists to keep.
• _keep columns are NOT NULL. They retain the identifiers after the FKs are blanked.

Both are verified behaviourally below rather than by reading the DDL back.

Two indexes, not one

The ticket specified idx_audit_team_membership_group. This also adds idx_audit_team_membership_user on (user_id_keep, id).

That is not an optimisation. With a single administrator the audit log has no peer readership, so the member's own view is the only place a password reset becomes visible to the account it was performed on — design §5.5 leans on this directly. It is a first-class read path, and without the index it is a table scan. Adding it later would cost another three-dialect migration, so it goes in now.

The role column is dropped

The ticket's DDL carried "role" TEXT DEFAULT NULL. It is a leftover from the pre-reversal design and contradicts the settled model: PUT-1699 states there is no role column, and the design doc says roles do not exist in eight separate places. PUT-1699's own reasoning applies — "a nullable column nobody reads is an invitation to start reading it inconsistently".

I removed it and reconciled the design doc, which had the same leftover in four other spots (§3 capability table, §10.5, the phase-2 scope list, and SR-4). The ticket DDL is updated to match.

share.holder_group_id

Mirrors holder_user_id from 0067/mysql_mig_22, CASCADE included — a share row is a listing entry, not the grant itself, so it should go when its group does.

⚠ The existing unique index does not constrain team shares at all. idx_share_holder_entry_issuer is (holder_user_id, fsentry_id, issuer_user_id), and a team share leaves holder_user_id NULL. All three engines treat NULLs as distinct in a unique index, so every team share trivially satisfies it. idx_share_holder_group_entry_issuer is what actually prevents a duplicate team share. Verified in both directions on sqlite and mysql below — distinct groups coexist, a genuine duplicate is refused.

---

Verification

Postgres is verified live for the first time in this stack. I ran a postgres:16 container, which makes PostgresDatabaseClient.integration.test.ts — this ticket's stated done-when — actually executable rather than deferred to CI.

postgres 16 — verified against a live server

$ PUTER_TEST_POSTGRES_URL=postgres://…@127.0.0.1:5433/puter \
    npx vitest run --config src/backend/vitest.config.ts \
    src/backend/clients/database/PostgresDatabaseClient.integration.test.ts

 Test Files  1 passed (1)
      Tests  2 passed (2)

The first of those two applies the whole native chain to an empty database twice, which is the idempotency check the ticket asks for.

Confirming the run was real, not the pg-mock fallback

The test silently falls back to an in-memory mock when PUTER_TEST_POSTGRES_URL is unset, and 2.7s looked fast for a 16-migration chain. Pointing it at a dead port must fail if the variable is honoured:

$ PUTER_TEST_POSTGRES_URL=postgres://…@127.0.0.1:9999/puter  # nothing listening
 Test Files  1 failed (1)
      Tests  2 failed (2)

It fails, so the passing run used the real server.

FK delete rules from the catalog

Chain applied into a scratch schema, then pg_constraint.confdeltype (n = SET NULL, c = CASCADE):

                 conname                  |     column      | del_rule
------------------------------------------+-----------------+----------
 audit_team_membership_actor_user_id_fkey | actor_user_id   | n
 audit_team_membership_group_id_fkey      | group_id        | n
 audit_team_membership_user_id_fkey       | user_id         | n
 share_holder_group_id_fkey               | holder_group_id | c

Matching the other two engines.

sqlite — verified

$ npx vitest run --config src/backend/vitest.config.ts src/backend/clients/database/
 Test Files  9 passed (9)
      Tests  33 passed (33)   # SqliteDatabaseClient.test.ts, was 30

$ npm run test:backend
 Test Files  250 passed | 24 skipped (274)
      Tests  6693 passed | 26 skipped (6719)   # was 6690

Three tests added:

• applies the audit table and group-share columns from 0078 — exact column list and all five indexes.
• keeps an audit row after its group is deleted, blanking only the FK — inserts a reset_member_password row, deletes the group, asserts group_id is NULL while group_id_keep and action survive.
• constrains team shares that the user-holder index cannot — two distinct-group shares of one file coexist; the duplicate is rejected.

The second one is behavioural rather than a DDL string assertion because sqlite does enforce foreign keys here — 0043_add_dt.sql ends with PRAGMA foreign_keys = ON and never restores it, so the setting persists for the connection. Worth knowing; it is easy to assume otherwise since nothing in SqliteDatabaseClient.ts sets it.

mysql 8.4 — verified against a live server

Throwaway database, not the local dev puter DB.

Chain, idempotency, and table shape

chain mysql_mig_1..28: applied clean

re-run 28 (mysql has no applied-state tracking):
  pass 1: clean
  pass 2: clean

COLUMN_NAME     COLUMN_TYPE    IS_NULLABLE
id              int unsigned   NO
group_id        int unsigned   YES     <- nullable FK
group_id_keep   int unsigned   NO      <- NOT NULL keep
user_id         int unsigned   YES
user_id_keep    int unsigned   NO
actor_user_id   int unsigned   YES
action          varchar(255)   NO
reason          varchar(255)   YES
created_at      timestamp      NO

leftover routines matching '%group_share%': 0

FK rules and SET NULL behaviour

CONSTRAINT_NAME                  COLUMN_NAME      DELETE_RULE  UPDATE_RULE
fk_audit_team_membership_actor   actor_user_id    SET NULL     CASCADE
fk_audit_team_membership_group   group_id         SET NULL     CASCADE
fk_audit_team_membership_user    user_id          SET NULL     CASCADE
share_holder_group_fk            holder_group_id  CASCADE      CASCADE

-- insert an audit row, then delete the group it points at:
group_id  group_id_keep  user_id_keep  action
NULL      1              1             reset_member_password

The row outlived its group, and still names which workspace and account it concerned.

The share indexes actually constrain

INDEX_NAME                           NON_UNIQUE  SEQ  COLUMN_NAME
idx_share_holder_group                        1    1  holder_group_id
idx_share_holder_group                        1    2  id
idx_share_holder_group_entry_issuer           0    1  holder_group_id
idx_share_holder_group_entry_issuer           0    2  fsentry_id
idx_share_holder_group_entry_issuer           0    3  issuer_user_id

two distinct-group shares of the same file: 2 inserted
duplicate (same group, file, issuer):
  ERROR 1062 (23000): Duplicate entry '2-1-1'
                      for key 'share.idx_share_holder_group_entry_issuer'

Both rows have holder_user_id NULL, so the pre-existing unique index permitted them — the new one is doing the work.

The throwaway database was dropped afterwards and the dev puter DB left untouched (audit table in dev puter db: 0).

typecheck

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

---

---

Review follow-up (fix: index the audit foreign keys so deletes do not scan)

The audit FK columns were unindexed on sqlite and postgres. All three are ON DELETE SET NULL, so every user or group delete has to find its child rows — and with no index that means scanning the whole append-only audit table, twice for a user since actor_user_id also points at user. Account hard-delete is a named operation in this feature, so the table gets scanned on exactly the path that matters, and it gets slower as the audit grows.

mysql already had these, because InnoDB requires an index on a FK column — which is why the gap was invisible from the mysql side. Its KEYs are renamed from fk_* to the same idx_*_fk scheme so all three dialects now read identically.

Three-dialect index parity, dumped from live engines after the fix:

Index	sqlite	mysql	postgres
idx_audit_team_membership_group	group_id_keep, id	same	same
idx_audit_team_membership_user	user_id_keep, id	same	same
idx_audit_team_membership_{group,user,actor}_fk	the FK columns	same	same
idx_share_holder_group	holder_group_id, id	same	same
idx_share_holder_group_entry_issuer	UNIQUE, 3 cols	same	same

Also fixed AND/OR precedence in the index-list assertion. WHERE type = 'index' AND name LIKE ? OR name LIKE ? binds as (type='index' AND …) OR (…), leaving the second pattern unscoped to indexes — it passed by luck and would have matched a table or trigger with that prefix.

$ npm run test:backend
 Test Files  250 passed | 24 skipped (274)
      Tests  6701 passed | 26 skipped (6727)

Rebased onto current main.

Not fixed here — needs a decision

• A soft-deleted workspace burns its handle forever. deleted_at plus a global unique index means the handle can never be reused, and mysql has no partial indexes, so this needs a deliberate answer (null the handle on delete, suffix it, or accept permanent reservation) rather than an index tweak. PUT-1702 territory.
• share.recipient_email is NOT NULL in all three dialects, but a team share has no recipient email — the new test has to invent team@test.local to insert a row, which is the smell. Either it becomes nullable or we commit to a documented sentinel.

---

⚠ Changed since approval

Approved at 51bf3e791. Renumbered only: 0078_team-audit-and-group-shares.sql
/ mysql_mig_32 / postgres_mig_21. The table, its five indexes and
share.holder_group_id are unchanged.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	93.97% 🟰 ±0%	27331 / 29084
🔵	Statements	92.07% ⬇️ -0.01%	29628 / 32177
🔵	Functions	90.39% 🟰 ±0%	4910 / 5432
🔵	Branches	80.91% 🟰 ±0%	19720 / 24371

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/backend/clients/database/SqliteDatabaseClient.ts	89.14% 🟰 ±0%	57.45% 🟰 ±0%	81.6% 🟰 ±0%	95.58% 🟰 ±0%	6, 9, 13, 16, 24-26, 32, 33, 34, 38-39, 44, 45, 46, 47, 48, 242-244, 492-495, 510

Generated in workflow #1403 for commit 64db3fa by the Vitest Coverage Report Action

[Salazareo]

looks fine, but I think we should expire and clean out audits after some amount of time

[jfcastro92]

Local validation (post-merge)

First execution of this migration's mysql DDL anywhere — the automated suite runs
sqlite and postgres only.

Boot 1

[mysql] running migrations from .../migrations/mysql: 34 file(s)
[mysql] applied mysql_mig_34.sql (6 statements)     <- this PR

No errors in the boot log.

Schema afterwards

audit_team_membership table present:                  1
audit idx_*_fk indexes:                               3   (group / user / actor)
share.holder_group_id column present:                 1

The three idx_*_fk indexes are the review follow-up in this PR: the FK columns
are ON DELETE SET NULL, so without them a user or group delete scans the whole
append-only audit table — twice for a user, since actor_user_id also points at
user. All three exist on mysql, matching sqlite and postgres.

Boot 2 — idempotency

diff <(grep mysql_mig boot1.log) <(grep mysql_mig boot2.log)
  -> identical
errors / "already exists" in boot 2: none

distinct indexes on audit_team_membership: 6   (unchanged — no duplicates created)

The guarded INFORMATION_SCHEMA.STATISTICS checks hold on a database that already
has the table and every index, which is the case that matters given mysql re-runs
every statement on every boot.

The table is written by the real code path. After provisioning three seats and
a disable/enable cycle through /teams, read back via GET /teams/<uid>/audit:

{"action":"enable",    "username":"acmeseat1","actor_username":"tmowner","created_at":"2026-09-03T23:29:40.000Z"}
{"action":"disable",   "username":"acmeseat1","actor_username":"tmowner","created_at":"2026-09-03T23:29:39.000Z"}
{"action":"provision", "username":"acmeseat3","actor_username":"tmowner","created_at":"2026-09-03T23:27:29.000Z"}
{"action":"provision", "username":"acmeseat2","actor_username":"tmowner","created_at":"2026-09-03T23:27:29.000Z"}
{"action":"provision", "username":"acmeseat1","actor_username":"tmowner","created_at":"2026-09-03T23:27:28.000Z"}

Actor attribution, target and timestamp all populate, ordered newest first.