From 77ab14143ae8ec616711bf310d7841e09dd3b680 Mon Sep 17 00:00:00 2001
From: Oskar Otwinowski <oskar@trigger.dev>
Date: Fri, 10 Jul 2026 18:23:08 +0200
Subject: [PATCH] fix(webapp): keep the last Owner on directory-sync role
 changes

Applying a directory-sync effect that would demote the org's last Owner (a
group remap, or a provision) previously threw and 500'd the settings save. Now
rbac.setUserRole reports code:"last_owner" and applyEffect skips just that
member (they keep Owner) while the rest of the batch applies.

Adds the machine-readable RoleAssignmentResult.code to the plugin contract so
callers can tell the last-owner guard apart from a real failure.
---
 .../services/directorySyncEffects.server.ts   | 23 ++++++++++++++++++-
 packages/plugins/src/rbac.ts                  |  8 ++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/apps/webapp/app/services/directorySyncEffects.server.ts b/apps/webapp/app/services/directorySyncEffects.server.ts
index f82cf66737..24e1dade5a 100644
--- a/apps/webapp/app/services/directorySyncEffects.server.ts
+++ b/apps/webapp/app/services/directorySyncEffects.server.ts
@@ -95,7 +95,19 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
           roleId: effect.roleId,
         });
         if (!result.ok) {
-          throw retryableEffectError(`directorySync provision setUserRole failed: ${result.error}`);
+          // The org must keep one Owner: skip the role overwrite for the last
+          // Owner (they keep Owner) instead of failing the whole batch. Applies
+          // to a directory burst and to a dashboard group remap alike.
+          if (result.code === "last_owner") {
+            logger.info("directorySync: kept last Owner, skipped provision role overwrite", {
+              userId,
+              organizationId: effect.organizationId,
+            });
+          } else {
+            throw retryableEffectError(
+              `directorySync provision setUserRole failed: ${result.error}`
+            );
+          }
         }
       }
       return;
@@ -107,6 +119,15 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
         roleId: effect.roleId,
       });
       if (!result.ok) {
+        // Keeping the org's last Owner is expected, not a failure — skip this
+        // one member and let the rest of the remap apply (no server error).
+        if (result.code === "last_owner") {
+          logger.info("directorySync: kept last Owner, skipped set_role", {
+            userId: effect.userId,
+            organizationId: effect.organizationId,
+          });
+          return;
+        }
         throw retryableEffectError(`directorySync set_role failed: ${result.error}`);
       }
       return;
diff --git a/packages/plugins/src/rbac.ts b/packages/plugins/src/rbac.ts
index cd8c2d03f1..b31abd1215 100644
--- a/packages/plugins/src/rbac.ts
+++ b/packages/plugins/src/rbac.ts
@@ -422,7 +422,13 @@ export interface RoleBaseAccessController {
 export type RoleMutationResult = { ok: true; role: Role } | { ok: false; error: string };
 
 // Result for assignment / deletion mutations that don't return a value.
-export type RoleAssignmentResult = { ok: true } | { ok: false; error: string };
+// `code` is an optional machine-readable reason so callers can branch on
+// expected outcomes (e.g. `last_owner`, the guard that keeps an org from
+// losing its final Owner) instead of matching the free-text `error`.
+export type RoleAssignmentErrorCode = "last_owner";
+export type RoleAssignmentResult =
+  | { ok: true }
+  | { ok: false; error: string; code?: RoleAssignmentErrorCode };
 
 import type { PluginDatabaseConfig } from "./databaseConfig.js";