marketplace: minesweeper in the catalogue

Without an entry the plugin is invisible to every server but this one — the
catalogue is what a remote host reads, and the bundled constant is only the
fallback when marketplace.officer.dev cannot be reached.

Its git remote was the tailnet address, which marketplace-sync would have
published verbatim: a clone URL that works on this network and nowhere else,
failing at authentication in a way that reads as a broken plugin rather than a
wrong catalogue. Repointed at gitea.officer.dev, which is the rule the sync
script's own comment already spells out.

Synced. 18/18, and the entry is live with the right source, permissions and six
osDependencies; the repo answers an anonymous https clone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043

src/databases/officer_db/src/auth/index.ts

@@ -20,10 +20,11 @@ export {
   cleanupExpiredTokens,
 } from './queries';
 
-// Exported as a value, not just a type: the API and the UI need to enumerate the roles, and the
-// column definition is the only place that list should exist.
-export { USER_ROLES, OWNER_USER_ID } from './schema';
+export { OWNER_USER_ID } from './schema';
 
+// A role list is no longer enumerable at the type level — see the comment on `UserRole` in schema.ts.
+// The API and the UI enumerate roles by querying the `roles` table (`listRoles` in ../roles), not by
+// importing a value from here.
 export type { UserRole } from './schema';
 export { listProvisionableUsers } from './queries';
 export type { ProvisionableUser } from './queries';

src/databases/officer_db/src/auth/schema.ts

@@ -1,25 +1,28 @@
-import { pgTable, serial, text, integer, timestamp, index, uniqueIndex, check } from 'drizzle-orm/pg-core';
+import { pgTable, serial, text, integer, timestamp, index, uniqueIndex, check, foreignKey } from 'drizzle-orm/pg-core';
 import { sql } from 'drizzle-orm';
+import { roles } from '../roles/schema';
 
 // The bootstrap account. `id` is a serial starting at 1 and bootstrap is gated on an empty user table,
 // so the first account created is always 1. Exported so the CHECK below and the delete guard in
 // queries/auth.ts state the number once between them.
 export const OWNER_USER_ID = 1;
 
-// Roles, in descending authority. Exported as a value so the API and the UI can enumerate them from
-// the same place the column is defined — a second hand-written list of role names is how they drift.
-//
-// A text column with a TS enum rather than a Postgres enum type, matching `status` below: the set is
-// still settling, and adding a role should be a one-line schema change that `db:push` applies, not an
-// ALTER TYPE. Postgres will not reject an unknown string — the TS type is the guard.
-//
-// 'Super Admin' is the server owner, and this column is the ONLY thing that says so. isSuperAdmin()
-// and getOwnerUser() both read it; there is no SUPER_ADMIN_EMAIL and no "first account wins" fallback
-// any more. Two consequences worth holding on to: bootstrap must set it explicitly on the first
-// account, and demoting the last Super Admin would leave the platform with no owner. The CHECK on the
-// table below stops that for the bootstrap account specifically: user 1 cannot be anything else.
-export const USER_ROLES = ['Super Admin', 'Admin', 'Member', 'Developer'] as const;
-export type UserRole = (typeof USER_ROLES)[number];
+/**
+ * Was a TS union (`(typeof USER_ROLES)[number]`) backed by a hardcoded four-role array. Since
+ * docs/roles-and-teams.md phase 2 part 4, roles are data — the `roles` table, administered through
+ * `/api/roles` — and `users.role` is a foreign key onto it rather than a column typed to a fixed set. A
+ * role this column can legally hold is therefore not enumerable at the type level any more, so `UserRole`
+ * is plain `string` and the compile-time guard it used to provide moves to a runtime check at the write
+ * path: `getRole(name)` returning a row before a caller is allowed to assign or grant that name. See
+ * `create-user.ts`, `manage-users.ts` and `permissions-routes.ts`.
+ *
+ * 'Super Admin' is still the server owner's role by name, and this column is the ONLY thing that says so
+ * — isSuperAdmin() and getOwnerUser() both read it. Two consequences worth holding on to: bootstrap must
+ * set it explicitly on the first account, and demoting the last Super Admin would leave the platform with
+ * no owner. The CHECK on the table below stops that for the bootstrap account specifically: user 1 cannot
+ * be anything else.
+ */
+export type UserRole = string;
 
 export const users = pgTable(
   'users',
@@ -31,8 +34,9 @@ export const users = pgTable(
       .notNull()
       .default('Unverified'),
     // Defaults to the least privileged role so a row created by any path that does not think about
-    // authorisation cannot accidentally mint an admin.
-    role: text('role', { enum: USER_ROLES }).notNull().default('Member'),
+    // authorisation cannot accidentally mint an admin. Foreign key onto `roles.name` is declared below,
+    // in the table's third argument — see `fk_users_role`.
+    role: text('role').notNull().default('Member'),
     name: text('name'),
     /**
      * Required, and unique case-INSENSITIVELY — see the index below.
@@ -110,6 +114,20 @@ export const users = pgTable(
       'ck_users_owner_is_super_admin',
       sql`${table.id} <> ${sql.raw(String(OWNER_USER_ID))} OR ${table.role} = ${sql.raw("'Super Admin'")}`,
     ),
+    // docs/roles-and-teams.md D5 and phase 2 part 2. Named explicitly rather than left to drizzle's
+    // generated name — see src/databases/CLAUDE.md → "Composite keys" on why an over-long generated FK
+    // name gets truncated by Postgres and then re-created on every push; named up front so this one is
+    // never at risk of it regardless of length.
+    //
+    // ON UPDATE CASCADE only, no ON DELETE: a role cannot be deleted while any account holds it
+    // (`deleteRole` in roles/queries.ts enforces D4), so there is no delete path for this to react to —
+    // and leaving it at Postgres's default (effectively NO ACTION) makes the constraint itself a backstop
+    // for that guard rather than silently nulling or cascading a delete the application never intended.
+    foreignKey({
+      name: 'fk_users_role',
+      columns: [table.role],
+      foreignColumns: [roles.name],
+    }).onUpdate('cascade'),
   ],
 );
 

src/databases/officer_db/src/permissions/schema.ts

@@ -1,6 +1,6 @@
-import { pgTable, serial, text, timestamp, uniqueIndex, check } from 'drizzle-orm/pg-core';
+import { pgTable, serial, text, timestamp, uniqueIndex, check, foreignKey } from 'drizzle-orm/pg-core';
 import { sql } from 'drizzle-orm';
-import { USER_ROLES } from '../auth/schema';
+import { roles } from '../roles/schema';
 
 // What a ROLE may reach. The subject of a grant is a role, never a user.
 //
@@ -8,8 +8,8 @@ import { USER_ROLES } from '../auth/schema';
 // look more flexible and are the obvious next request. Per-user grants make "what can this person do" a
 // question you answer by reading rows, and "what can a Member do" a question with no answer at all. Roles
 // keep both answerable: onboarding is picking a role, and auditing is reading one short table. When the
-// answer really has to differ for one person, that person needs a role, and adding one is a line in
-// USER_ROLES.
+// answer really has to differ for one person, that person needs a role, and adding one is a row in
+// `roles` (see roles/schema.ts and /api/roles).
 //
 // A MISSING ROW MEANS NO ACCESS. There is no row that denies; absence denies. This is what makes the table
 // safe to read as the whole truth — an empty table is a platform where members can reach nothing but their
@@ -27,7 +27,8 @@ export const rolePermissions = pgTable(
   'role_permissions',
   {
     id: serial('id').primaryKey(),
-    role: text('role', { enum: USER_ROLES }).notNull(),
+    // Foreign key onto `roles.name` is declared below — see `fk_role_permissions_role`.
+    role: text('role').notNull(),
     /** A permission key from the registry. Validated by the API, not by a constraint — see above. */
     permission: text('permission').notNull(),
     /**
@@ -43,16 +44,6 @@ export const rolePermissions = pgTable(
     // re-creates them on every push, which stops db:push on an unanswerable truncate prompt. See
     // src/databases/CLAUDE.md → "Composite keys".
     uniqueIndex('uq_role_permissions_role_permission').on(table.role, table.permission),
-    // Postgres will not reject an unknown string in a text column just because TypeScript narrowed it, so
-    // the legal sets are stated here too. sql.raw for the literals: an interpolated JS string binds as a
-    // parameter and Postgres refuses a parameter inside a CHECK, which breaks push for the WHOLE schema.
-    check(
-      'ck_role_permissions_role',
-      sql`${table.role} IN (${sql.join(
-        USER_ROLES.map((r) => sql.raw(`'${r}'`)),
-        sql`, `,
-      )})`,
-    ),
     check(
       'ck_role_permissions_level',
       sql`${table.level} IN (${sql.join(
@@ -64,5 +55,19 @@ export const rolePermissions = pgTable(
     // short-circuits every check. A 'Super Admin' row here would be inert, and an inert row that looks
     // meaningful is worse than no row: someone would eventually revoke one and expect it to do something.
     check('ck_role_permissions_not_owner', sql`${table.role} <> ${sql.raw("'Super Admin'")}`),
+    // docs/roles-and-teams.md D5 and phase 2 part 2. This REPLACES `ck_role_permissions_role`, which used
+    // to list the four built-in role names literally (`role IN ('Super Admin', 'Admin', ...)`). That check
+    // was already wrong the moment role CRUD (phase 2 part 3, `/api/roles`) could create a role at runtime
+    // — a grant for a new role would have been refused by a constraint enforcing a snapshot of a list that
+    // no longer described the actual role set. A foreign key onto the actual table is the correct
+    // enforcement of "must be a real role" and needs no updating when a role is added.
+    //
+    // Named explicitly for the same reason as `fk_users_role` — see the comment there and
+    // src/databases/CLAUDE.md → "Composite keys".
+    foreignKey({
+      name: 'fk_role_permissions_role',
+      columns: [table.role],
+      foreignColumns: [roles.name],
+    }).onUpdate('cascade'),
   ],
 );

src/databases/officer_db/src/roles/queries.ts

@@ -1,7 +1,6 @@
 import { and, asc, eq, sql } from 'drizzle-orm';
 import { db } from '../db';
 import { users } from '../auth/schema';
-import type { UserRole } from '../auth/schema';
 import { roles } from './schema';
 
 export type RoleRow = {
@@ -69,12 +68,12 @@ export async function seedRole(row: {
 // shares: it re-checks its own invariant inside the write, not just before it, so a race between the
 // check and the statement fails closed rather than silently succeeding.
 
-/** How many accounts hold this role. One definition, so rename and delete cannot disagree on "in use". */
+/** How many accounts hold this role. Used by `deleteRole` — D4 refuses to delete a role anyone holds. */
 async function roleHolderCount(name: string): Promise<number> {
   const [row] = await db
     .select({ count: sql<number>`count(*)::int` })
     .from(users)
-    .where(eq(users.role, name as UserRole));
+    .where(eq(users.role, name));
   return row?.count ?? 0;
 }
 
@@ -101,7 +100,7 @@ export async function createRole(input: { name: string; description?: string | n
 
 export type RenameRoleResult =
   | { ok: true; role: RoleRow }
-  | { ok: false; reason: 'not-found' | 'system' | 'invalid-name' | 'duplicate' | 'in-use'; holderCount?: number };
+  | { ok: false; reason: 'not-found' | 'system' | 'invalid-name' | 'duplicate' };
 
 /**
  * Rename a role. Refuses outright on `isSystem` (D4 in docs/roles-and-teams.md): `rolePermitsDevTools()`
@@ -109,10 +108,12 @@ export type RenameRoleResult =
  * `'Member'`. Renaming one of those four is a code change across three call sites that has not happened,
  * not a data edit — so this refuses rather than quietly breaking them.
  *
- * `users.role` and `role_permissions.role` are NOT yet foreign keys onto this table (phase 2 part 2 of the
- * plan, shipped as its own release), so this UPDATE does not cascade anywhere today — it only relabels the
- * `roles` row itself, leaving every holder and every grant still naming the OLD name. Once the FK lands
- * with `ON UPDATE CASCADE`, this same statement starts moving them automatically and nothing here changes.
+ * `users.role` and `role_permissions.role` are foreign keys onto this table with `ON UPDATE CASCADE`
+ * (phase 2 part 2), so the `UPDATE` below moves every holder and every grant automatically, in the same
+ * statement, as Postgres's own job rather than this function's. There used to be a guard here refusing the
+ * rename while anyone held the role, because before the FK existed a rename would have orphaned them —
+ * silently pointing every holder at a name that no longer existed. That guard is gone now that the
+ * database itself does the moving.
  */
 export async function renameRole(from: string, to: string): Promise<RenameRoleResult> {
   const name = to.trim();
@@ -123,24 +124,6 @@ export async function renameRole(from: string, to: string): Promise<RenameRoleRe
   if (existing.isSystem) return { ok: false, reason: 'system' };
   if (name === from) return { ok: true, role: existing };
 
-  /*
-    Refuse a rename while anybody holds it, for exactly as long as there is no foreign key.
-
-    `users.role` is a plain text column today — phase 2 part 2 of `docs/roles-and-teams.md` ships the
-    foreign key with ON UPDATE CASCADE as its own release — so renaming this row would leave every holder
-    pointing at a name that no longer exists.
-
-    It happens to be unreachable right now: `manage-users.ts` validates an incoming role against the
-    `USER_ROLES` union, so a role created at runtime cannot be assigned and a non-system role has no
-    holders. That is safety by accident, and it evaporates the moment part 4 makes `UserRole` a plain
-    string. This guard means those two parts can land in either order without opening a window.
-
-    Delete it when the cascade exists, and not before — the cascade then does this job properly, by moving
-    the holders rather than refusing.
-  */
-  const held = await roleHolderCount(from);
-  if (held > 0) return { ok: false, reason: 'in-use', holderCount: held };
-
   const clash = await getRole(name);
   if (clash) return { ok: false, reason: 'duplicate' };
 
@@ -173,14 +156,12 @@ export async function deleteRole(name: string): Promise<DeleteRoleResult> {
   if (existing.isSystem) return { ok: false, reason: 'system' };
 
   return db.transaction(async (tx) => {
-    // `name` is not necessarily one of the four literal roles the column's TS type enumerates — this
-    // function accepts any role that exists as a ROW, which by definition this one is not (yet, if it
-    // never was). The cast is type-level only: `role` is a plain `text` column, so drizzle emits the same
-    // `role = $1` whether or not the value belongs to that union.
+    // Inline rather than a call to `roleHolderCount`: this one has to run inside the same transaction as
+    // the delete below, so a role cannot be handed to someone in the gap between counting and deleting.
     const [holders] = await tx
       .select({ count: sql<number>`count(*)::int` })
       .from(users)
-      .where(eq(users.role, name as UserRole));
+      .where(eq(users.role, name));
     const count = holders?.count ?? 0;
     if (count > 0) return { ok: false, reason: 'in-use', holderCount: count };