From 6ffe459a2b02269b440ea0dbe70721b322d92fd2 Mon Sep 17 00:00:00 2001
From: Oskar Otwinowski <oskar@trigger.dev>
Date: Thu, 9 Jul 2026 18:04:36 +0200
Subject: [PATCH] chore(webapp): trim comments in directorySyncEffects

Condense the kept rationale comments (logLevel/warn, last-Owner dedup, role
overwrite) and drop the obvious function-header comments that just restated
the code. No behavior change.
---
 .../services/directorySyncEffects.server.ts   | 26 +++++--------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/apps/webapp/app/services/directorySyncEffects.server.ts b/apps/webapp/app/services/directorySyncEffects.server.ts
index 54934c706d4..f82cf667378 100644
--- a/apps/webapp/app/services/directorySyncEffects.server.ts
+++ b/apps/webapp/app/services/directorySyncEffects.server.ts
@@ -11,21 +11,15 @@ import { createPlatformNotification } from "~/services/platformNotifications.ser
 
 const LAST_OWNER_NOTIFICATION_TITLE = "Directory Sync: last Owner protected";
 
-// Directory-sync effects are idempotent and the accounts-webhook worker retries
-// the whole event (maxAttempts: 5). A single failed attempt is therefore an
-// expected, self-healing condition rather than an alert-worthy error — the most
-// common case is a role assignment losing a serializable race during a backfill
-// burst (the plugin already retries that internally; the worker retry mops up
-// the rest). Tag the thrown error with `logLevel: "warn"` so the worker logs it
-// at warn instead of error (see redis-worker `processItem`), keeping it visible
-// for triage without paging as an error on every transient retry.
+// Effects are idempotent and the worker retries, so a single failed attempt is
+// transient (usually a serializable-conflict retry), not an alert. `logLevel`
+// makes the worker log it at warn instead of paging.
 function retryableEffectError(message: string): Error {
   return Object.assign(new Error(message), { logLevel: "warn" as const });
 }
 
-// Raise a user-scoped, deduped notification when the directory tried to
-// remove the org's last Owner. We keep the member and tell the Owner what to
-// do; a single undismissed notification is enough (don't spam on every retry).
+// Deduped notification when the directory tried to remove the org's last Owner:
+// we keep the member, and one undismissed notification is enough (no retry spam).
 async function notifyLastOwnerProtected(userId: string, organizationId: string): Promise<void> {
   const existing = await prisma.platformNotification.findFirst({
     where: {
@@ -72,9 +66,6 @@ async function notifyLastOwnerProtected(userId: string, organizationId: string):
   }
 }
 
-// Apply one directory-sync membership effect against public.* tables. The
-// plugin owns all enterprise.* state and never writes here; this is the only
-// path that mutates User / OrgMember / roles / tokens from a directory event.
 async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
   switch (effect.kind) {
     case "provision": {
@@ -95,8 +86,8 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
         source: "directory_sync",
       });
 
-      // Directory is authoritative for role: overwrite even for an existing
-      // member (ensureOrgMember only sets the role on first create).
+      // Directory owns the role: overwrite even an existing member
+      // (ensureOrgMember only sets it on create).
       if (effect.roleId) {
         const result = await rbac.setUserRole({
           userId,
@@ -133,9 +124,6 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
   }
 }
 
-// Apply all effects from a processed directory-sync webhook. Effects are
-// idempotent, so a worker retry that re-applies them converges. A throw
-// propagates to the worker for retry.
 export async function applyDirectorySyncEffects(effects: DirectorySyncEffect[]): Promise<void> {
   for (const effect of effects) {
     await applyEffect(effect);