fix(identity): make admin remove-user atomic, cascade sys_member, and map DELETE_RESTRICTED to a 409

.changeset/olive-moons-shave.md

@@ -0,0 +1,12 @@
+---
+'@objectstack/platform-objects': patch
+'@objectstack/plugin-auth': patch
+---
+
+Fix `POST /api/v1/auth/admin/remove-user`, which could never succeed and left the identity un-authenticatable when it failed.
+
+Three compounding problems on the better-auth admin removal path:
+
+- **`sys_member.user_id` declared no `deleteBehavior`.** A `lookup` defaults to `set_null`, and the engine escalates a defaulted `set_null` on a REQUIRED foreign key to `restrict` — so the membership every user gets at sign-up (and, since the invitation-adoption change, keeps after accepting an invitation) vetoed every `sys_user` delete. The field now declares `deleteBehavior: 'cascade'`. The last-administrator invariant is unaffected: it is enforced by a `beforeDelete` hook on `sys_member`, and the engine's cascade recurses through the public `delete()`, so that hook still runs.
+- **The removal was not atomic.** better-auth deletes the sessions, then the accounts, then the user, in three calls with no transaction, so anything refusing the last one left the credential rows deleted and the user row behind — an identity still on the org roster that can no longer sign in. Subject-erasure requests now run inside one engine transaction and roll back as a unit. Datasources whose driver has no transaction support keep the previous behaviour and log the engine's existing warning.
+- **A referential refusal reached the client as an HTTP 500 with an empty body.** The auth adapter mapped engine validation errors and policy refusals to better-auth `APIError`s but not referential ones, so a `DELETE_RESTRICTED` escaped unmapped. It now surfaces as a structured 409 carrying the dependent object, the dependent count and the remedy.

packages/platform-objects/src/identity/sys-member.object.ts

@@ -164,6 +164,27 @@ export const SysMember = ObjectSchema.create({
  user_id: Field.lookup('sys_user', {
  label: 'User',
  required: true,
+ // [#7724] A membership without its user is meaningless, so deleting the
+ // user takes its memberships with it. This must be DECLARED: a `lookup`
+ // defaults to `set_null`, and the engine escalates a *defaulted*
+ // `set_null` on a REQUIRED foreign key to `restrict` (you cannot null a
+ // NOT NULL column). That escalation vetoed every `sys_user` delete on any
+ // deployment where the membership reconciler had run — i.e. all of them,
+ // since `reconcile-membership.ts` binds every user to the default org at
+ // sign-up, and (since #7796) invitation acceptance ADOPTS that same row.
+ // So `/admin/remove-user` could never succeed, and the operator could not
+ // clear the blocker by hand either: `enable.apiMethods` below is read-only.
+ //
+ // Audited before declaring it, because the engine's own error naming
+ // `deleteBehavior:'cascade'` is a suggestion, not an audit: nothing
+ // depends on the restrict. In particular it is NOT an accidental
+ // last-administrator guard — that invariant is enforced by a `beforeDelete`
+ // hook registered on `sys_member` itself (ADR-0024 D5.2,
+ // `last-admin-guard.ts`), and the engine's cascade recurses through the
+ // PUBLIC `delete()` precisely so the child's own hooks and events fire.
+ // The guard therefore still refuses a cascade that would take the last
+ // administrator's standing away; it simply refuses it one row deeper.
+ deleteBehavior: 'cascade',
  }),
 
  // [ADR-0108 / #3723] The framework's four roles — the WHOLE list. Nothing

packages/plugins/plugin-auth/src/auth-manager.ts

@@ -22,6 +22,7 @@ import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/secu
 import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai';
 import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js';
 import { runWithAuthActorScope, setAuthActorResolver } from './auth-actor-attribution.js';
+import { SESSION_ERASURE_PATHS } from './session-tombstone.js';
 import {
  invitationRoleCapFailure,
  isPlainMemberInvitation,
@@ -165,6 +166,26 @@ function installWebContainerRequestStatePolyfill(): void {
  }
 }
 
+/**
+ * [#7724] Carries better-auth's own error `Response` out through the engine
+ * transaction that must roll back because of it.
+ *
+ * better-auth's HTTP entrypoint CATCHES every fault and RETURNS a `Response` —
+ * it does not throw. A `try`/`catch`-shaped unit of work therefore sees a clean
+ * return on the exact path it exists to undo, commits, and the partial writes
+ * land anyway. So the failure signal has to be re-raised from the response
+ * status, and the response itself has to survive the throw that rolls the
+ * transaction back — that is the whole job of this class. It never escapes
+ * `runSubjectErasureAtomically`, which unwraps it back into the response the
+ * client was always going to get.
+ */
+class SubjectErasureRollback extends Error {
+ constructor(readonly response: Response) {
+ super(`subject-erasure unit of work rolled back (HTTP ${response.status})`);
+ this.name = 'SubjectErasureRollback';
+ }
+}
+
 function readBooleanEnv(name: string, legacyName?: string): boolean | undefined {
  const env = (globalThis as any)?.process?.env as Record<string, string | undefined> | undefined;
  const raw = env?.[name] ?? (legacyName ? env?.[legacyName] : undefined);
@@ -3069,9 +3090,28 @@ export class AuthManager {
  // costs nothing: the scope starts empty, the before-hook drops a resolver
  // in, and the session is looked up only if some write asks. Attribution
  // only — the authorization subject of those writes is unchanged (system).
- const response = await runWithAuthActorScope(() =>
- runWithRequestState(new WeakMap(), () => auth.handler(request)),
- );
+ // `await`, not a bare return: both scope helpers are generic over their
+ // callback, so the composed call is typed `Promise< Promise< Response > >`.
+ // The previous single call site flattened it with the `await` below.
+ const runHandler = async (): Promise<Response> =>
+ await runWithAuthActorScope(() =>
+ runWithRequestState(new WeakMap(), () => auth.handler(request)),
+ );
+
+ // [#7724] A subject-erasure request is ONE unit of work, and better-auth
+ // does not treat it as one: `internalAdapter.deleteUser` deletes the
+ // sessions, then the accounts, then the user, in three unrelated adapter
+ // calls with no transaction (verified in better-auth 1.7.0-rc.2 —
+ // `dist/db/internal-adapter.mjs` mentions no transaction at all). Anything
+ // that refuses the LAST of those three leaves the first two committed: the
+ // credential rows are gone, the `sys_user` row is not, and the deployment
+ // is left with an identity that still occupies the org roster and can no
+ // longer sign in. Nothing tells the operator, and there is no way back.
+ const endpointPath = this.betterAuthEndpointPath(request);
+ const response =
+ endpointPath !== undefined && SESSION_ERASURE_PATHS.has(endpointPath)
+ ? await this.runSubjectErasureAtomically(runHandler)
+ : await runHandler();
 
  if (response.status >= 500) {
  try {
@@ -3085,6 +3125,84 @@ export class AuthManager {
  return response;
  }
 
+ /**
+ * The better-auth endpoint path (`/admin/remove-user`) this request addresses,
+ * or `undefined` when it is not under the configured `basePath`.
+ *
+ * The same spelling better-auth's own `ctx.path` uses, so the sets keyed by it
+ * — `SESSION_ERASURE_PATHS`, the break-glass guard's path tests — are all
+ * talking about one thing.
+ */
+ private betterAuthEndpointPath(request: Request): string | undefined {
+ let pathname: string;
+ try {
+ pathname = new URL(request.url).pathname;
+ } catch {
+ return undefined;
+ }
+ const configured = this.config.basePath || '/api/v1/auth';
+ const base = (configured.startsWith('/') ? configured : `/${configured}`).replace(/\/+$/, '');
+ if (!pathname.startsWith(base)) return undefined;
+ const endpoint = pathname.slice(base.length).replace(/\/+$/, '');
+ return endpoint.startsWith('/') ? endpoint : undefined;
+ }
+
+ /**
+ * [#7724] Run a subject-erasure request as ONE unit of work: every write it
+ * makes commits together, or none of them do.
+ *
+ * Placed at the REQUEST seam rather than inside better-auth's route, because
+ * re-implementing `/admin/remove-user` here would duplicate its permission
+ * check, its self-removal check and its not-found check — and a duplicated
+ * security check is where the two copies drift apart. This wrapper reads no
+ * bodies and makes no authorization decision; better-auth's handler runs
+ * exactly as before, and the only thing added is the transaction it runs in.
+ *
+ * The engine's `transaction()` (ADR-0034) publishes its handle into the
+ * ambient store, so every adapter write on the way down joins it without the
+ * adapter knowing — which is why this needs no change in `objectql-adapter.ts`.
+ *
+ * Two declared limits, both inherited rather than introduced:
+ * - a datasource whose driver has no `beginTransaction` runs the callback
+ * with no transaction and no rollback (ADR-0119 D1). The engine warns once
+ * per driver. Failing CLOSED instead (`{ require: true }`) was considered
+ * and rejected: it would make user removal impossible on those datasources,
+ * which is the very defect this card is fixing.
+ * - side effects outside the datasource (a sent email, secondary-storage
+ * session state) are not transactional and are not undone by a rollback.
+ * `/admin/remove-user` sends nothing, so no path here relies on it.
+ */
+ private async runSubjectErasureAtomically(
+ run: () => Promise<Response>,
+ ): Promise<Response> {
+ const engine = this.config.dataEngine as
+ | (IDataEngine & {
+ transaction?: <T>(callback: (trxCtx: any, info: any) => Promise<T>) => Promise<T>;
+ })
+ | undefined;
+ // `transaction` is an ObjectQL capability, not an `IDataEngine` member — an
+ // engine without it (a test double, a foreign engine) keeps the previous
+ // behaviour rather than being refused.
+ if (typeof engine?.transaction !== 'function') return run();
+
+ try {
+ return await engine.transaction(async () => {
+ const response = await run();
+ // better-auth RETURNS its faults; see `SubjectErasureRollback`. Any 4xx/5xx
+ // means the erasure did not complete, so whatever part of it already
+ // landed must not survive. 2xx commits; so does the 302 that
+ // `/delete-user/callback` answers with on success.
+ if (response.status >= 400) throw new SubjectErasureRollback(response);
+ return response;
+ });
+ } catch (err) {
+ // The rollback has happened by the time this runs — hand the client the
+ // response better-auth composed, now with no partial writes behind it.
+ if (err instanceof SubjectErasureRollback) return err.response;
+ throw err;
+ }
+ }
+
  /**
  * Get the better-auth API for programmatic access
  * Use this for server-side operations (e.g., creating users, checking sessions)

packages/plugins/plugin-auth/src/objectql-adapter.test.ts

@@ -460,6 +460,92 @@ describe('withValidationErrorMapping – ObjectQL ValidationError → better-aut
  await expect(adapter.update()).rejects.toBe(boom);
  });
 
+ // [#7724] The third arm. A referential veto is raised by the ENGINE, well
+ // below the layers that know better-auth exists, so it carried neither the
+ // validation envelope nor the policy-refusal code and fell through to
+ // `throw err` — reaching an admin as a 500 with an EMPTY body for a refusal
+ // the engine had explained in full.
+ describe('a referential delete restriction (DELETE_RESTRICTED) → 409', () => {
+ // Faithful mimic of the engine's envelope (`packages/objectql/src/engine.ts`,
+ // ADR-0112 + #7307's message split).
+ const restricted = () => {
+ const err: any = new Error('Cannot delete User: 1 or more Member records still reference it.');
+ err.code = 'DELETE_RESTRICTED';
+ err.status = 409;
+ err.object = 'sys_user';
+ err.dependentObject = 'sys_member';
+ err.dependentCount = 3;
+ err.developerMessage =
+ 'Cannot delete sys_user: 3 dependent sys_member record(s) reference it via user_id ' +
+ "(user_id is required, so it cannot be cleared). Delete or reassign them first, " +
+ "or set deleteBehavior:'cascade' on sys_member.user_id.";
+ return err;
+ };
+
+ it('maps it to a 409 APIError instead of letting it escape as a bodyless 500', async () => {
+ const adapter = withValidationErrorMapping({
+ delete: async () => {
+ throw restricted();
+ },
+ });
+
+ let caught: any;
+ try {
+ await adapter.delete();
+ } catch (e) {
+ caught = e;
+ }
+
+ // Both halves of the envelope, per ADR-0112: a throw alone cannot tell
+ // "refused with the wrong envelope" apart from "refused correctly" —
+ // the unfixed path throws too, it just throws something better-auth
+ // cannot render.
+ expect(isAPIError(caught)).toBe(true);
+ expect(caught.statusCode).toBe(409);
+ expect(caught.body).toMatchObject({
+ code: 'DELETE_RESTRICTED',
+ message: 'Cannot delete User: 1 or more Member records still reference it.',
+ });
+ });
+
+ it('carries the structured half through — the remedy stays reachable', async () => {
+ // #7307's reasoning at the REST mapping, applied to this transport:
+ // dropping `developerMessage` here would move the defect rather than fix
+ // it, and it discloses nothing `dependentObject` does not already.
+ const adapter = withValidationErrorMapping({
+ delete: async () => {
+ throw restricted();
+ },
+ });
+
+ const caught: any = await adapter.delete().catch((e: unknown) => e);
+ expect(caught.body.dependentObject).toBe('sys_member');
+ expect(caught.body.dependentCount).toBe(3);
+ expect(caught.body.developerMessage).toContain("deleteBehavior:'cascade'");
+ });
+
+ it('omits the structured keys when the engine did not supply them', async () => {
+ // A bare `DELETE_RESTRICTED` must still map — the arm keys off `code`,
+ // not off the optional detail — and must not invent `dependentCount: 0`,
+ // which would read as "no dependents" on the error that exists to say
+ // there are some.
+ const bare: any = new Error('Cannot delete: dependent records exist');
+ bare.code = 'DELETE_RESTRICTED';
+ const adapter = withValidationErrorMapping({
+ delete: async () => {
+ throw bare;
+ },
+ });
+
+ const caught: any = await adapter.delete().catch((e: unknown) => e);
+ expect(caught.statusCode).toBe(409);
+ expect(caught.body.code).toBe('DELETE_RESTRICTED');
+ expect(caught.body).not.toHaveProperty('dependentObject');
+ expect(caught.body).not.toHaveProperty('dependentCount');
+ expect(caught.body).not.toHaveProperty('developerMessage');
+ });
+ });
+
  it('passes successful results through untouched and leaves non-function props alone', async () => {
  const adapter = withValidationErrorMapping({
  create: async (x: number) => x + 1,

packages/plugins/plugin-auth/src/objectql-adapter.ts

@@ -497,10 +497,51 @@ function isEnginePolicyRefusal(err: unknown): err is { code?: string; message?:
  return (err as { code?: unknown }).code === 'PERMISSION_DENIED';
 }
 
+/**
+ * [#7724] A REFERENTIAL refusal — the engine's `cascadeDeleteRelations` found
+ * dependent rows it may neither cascade nor null, so it vetoed the delete
+ * (`DELETE_RESTRICTED`, 409, ADR-0112).
+ *
+ * The third shape in this file, and the one that shows why the set had to be
+ * widened rather than left at two. The two arms above both map errors raised by
+ * code that *knows about better-auth* — the record validator and this package's
+ * own policy guards. A referential restrict is raised by the ENGINE, several
+ * layers below, and carries neither signature; `rethrowAsBetterAuthError` fell
+ * through to `throw err`, better-auth's router saw an unhandled fault, and the
+ * admin caller got a **500 with an empty body** for a refusal the engine had
+ * explained in full. The client is told nothing at all — not the status, not the
+ * dependent object, not the remedy.
+ *
+ * Mapped HERE, at the adapter, rather than at the REST transport: this is the
+ * seam where an engine error crosses into better-auth, so one arm covers every
+ * better-auth endpoint that deletes through the adapter. `rest-server.ts`'s
+ * `mapDataError` already maps the same code correctly for the generic data
+ * routes and is deliberately untouched — the two transports map the one engine
+ * error independently, exactly as they already do for the two arms above.
+ *
+ * The structured half of the envelope rides along unchanged (`developerMessage`
+ * / `dependentObject` / `dependentCount`), for the reason #7307 gives at the
+ * REST mapping: dropping the remedy at the transport moves the defect rather
+ * than fixing it, and the fields disclose nothing the envelope did not carry.
+ */
+function isReferentialDeleteRestriction(
+ err: unknown,
+): err is {
+ code?: string;
+ message?: string;
+ developerMessage?: string;
+ dependentObject?: string;
+ dependentCount?: number;
+} {
+ if (!err || typeof err !== 'object') return false;
+ return (err as { code?: unknown }).code === 'DELETE_RESTRICTED';
+}
+
 /**
  * Re-throw `err` as a better-auth `APIError` when it is an ObjectQL validation
- * failure or an engine policy refusal; otherwise re-throw it verbatim. Always
- * throws — the return type is `never`.
+ * failure (400), an engine policy refusal (403) or a referential delete
+ * restriction (409); otherwise re-throw it verbatim. Always throws — the return
+ * type is `never`.
  */
 async function rethrowAsBetterAuthError(err: unknown): Promise<never> {
  if (isObjectQLValidationError(err)) {
@@ -525,15 +566,31 @@ async function rethrowAsBetterAuthError(err: unknown): Promise<never> {
  code: 'PERMISSION_DENIED',
  });
  }
+ if (isReferentialDeleteRestriction(err)) {
+ const { APIError } = await import('better-auth/api');
+ throw new APIError('CONFLICT', {
+ message:
+ typeof err.message === 'string' && err.message.trim()
+ ? err.message
+ : 'Cannot delete: dependent records exist',
+ code: 'DELETE_RESTRICTED',
+ ...(typeof err.developerMessage === 'string' && err.developerMessage.length > 0
+ ? { developerMessage: err.developerMessage }
+ : {}),
+ ...(err.dependentObject ? { dependentObject: err.dependentObject } : {}),
+ ...(typeof err.dependentCount === 'number' ? { dependentCount: err.dependentCount } : {}),
+ });
+ }
  throw err;
 }
 
 /**
  * Wrap every function-valued method of a better-auth adapter so an ObjectQL
- * `ValidationError` (400) or an engine policy refusal (403) thrown from the
- * underlying engine surfaces as a 4xx `APIError` instead of an opaque 500.
- * Non-function properties pass through untouched, and every error that carries
- * neither signature is re-thrown verbatim.
+ * `ValidationError` (400), an engine policy refusal (403) or a referential
+ * delete restriction (409, #7724) thrown from the underlying engine surfaces as
+ * a 4xx `APIError` instead of an opaque 500. Non-function properties pass
+ * through untouched, and every error that carries none of those signatures is
+ * re-thrown verbatim.
  */
 export function withValidationErrorMapping<A extends Record<string, any>>(adapter: A): A {
  const out: Record<string, any> = {};