fix(onboarding): split persistAppOnboarding (cognitive complexity 31 → ≤15)

[rihoarvutikonto]

Sonar S3776 on persistAppOnboarding

After #3359, Sonar flags supabase/functions/_backend/public/app/put.ts (persistAppOnboarding, line 43):

Refactor this function to reduce its Cognitive Complexity from 31 to the 15 allowed.

Quality gate on #3359 passed with that 1 new issue. This split keeps the same lock order, in-transaction RBAC recheck, RLS role dance, and merge/complete SQL. put() is unchanged.

Helpers, each under the threshold:

Function	Role
onboardingWriteIdentity	auth / user / key
assertOnboardingWritePermission	app.update_settings then org.create_app under the lock
applyAppSettingsInTransaction	icon path + caller RLS settings write
completePendingOnboardingRow	need_onboarding = false
loadPersistedApp	no-patch return
mergeOnboardingPatch	merge_app_onboarding_setup + history + try_complete_pending_onboarding_if_setup_done
persistAppOnboardingTx	lock → authz → settings → complete → patch
persistAppOnboarding	pool + retryAppOnboardingWrite

Existing coverage that should still pass: tests/app-onboarding-v3-postgres.test.ts, tests/onboarding-progress-endpoint.unit.test.ts.

I cannot open a PR from this account (GitHub App cannot fork). Patch against current main:

--- a/supabase/functions/_backend/public/app/put.ts
+++ b/supabase/functions/_backend/public/app/put.ts
@@ -40,10 +40,187 @@
   | 'need_onboarding' | 'existing_app' | 'block_provider_infra_requests'
   | 'ios_store_url' | 'android_store_url'>
 
+type AppRow = Database['public']['Tables']['apps']['Row']
+type OnboardingPatch = NonNullable<ReturnType<typeof parseAppOnboardingPatch>>
+type OnboardingTx = Parameters<Parameters<ReturnType<typeof getDrizzleClient>['transaction']>[0]>[0]
+type LockedAppOnboarding = NonNullable<Awaited<ReturnType<typeof lockAppOnboardingForWrite>>>
+type PersistAppOnboardingResult = {
+  app: AppRow
+  completed: boolean
+  historyChanges: ReturnType<typeof getAppOnboardingStepHistoryChanges>
+}
+
+function onboardingWriteIdentity(
+  c: Context<MiddlewareKeyVariables>,
+  apikey: Database['public']['Tables']['apikeys']['Row'],
+) {
+  const auth = c.get('auth')
+  const userId = auth?.userId ?? apikey.user_id
+  const key = auth?.apikey?.key ?? c.get('capgkey') ?? (auth?.authType === 'jwt' ? null : apikey.key)
+  return { auth, userId, key }
+}
+
+async function assertOnboardingWritePermission(
+  c: Context<MiddlewareKeyVariables>,
+  tx: OnboardingTx,
+  appId: string,
+  ownerOrg: string,
+  apikey: Database['public']['Tables']['apikeys']['Row'],
+  settings?: AppSettings,
+) {
+  const { userId, key } = onboardingWriteIdentity(c, apikey)
+  const canUpdateSettings = await checkPermissionPg(c, 'app.update_settings', { appId }, tx, userId, key)
+  if (!canUpdateSettings && (settings
+    || !(await checkPermissionPg(c, 'org.create_app', { orgId: ownerOrg }, tx, userId, key)))) {
+    throw quickError(401, 'cannot_access_app', 'You can\'t access this app', { app_id: appId })
+  }
+}
+
+async function applyAppSettingsInTransaction(
+  c: Context<MiddlewareKeyVariables>,
+  tx: OnboardingTx,
+  appId: string,
+  current: LockedAppOnboarding,
+  apikey: Database['public']['Tables']['apikeys']['Row'],
+  settings: AppSettings,
+): Promise<{ app: AppRow, completed: boolean }> {
+  const { auth, userId, key } = onboardingWriteIdentity(c, apikey)
+  if (settings.icon_url) {
+    settings = { ...settings, icon_url: resolveWritableImageValue(settings.icon_url, { orgId: current.owner_org, appId }, getStorageAllowedOrigins(c)) ?? undefined }
+    if (!settings.icon_url)
+      throw simpleError('invalid_icon_path', 'Icon path must belong to this app organization')
+  }
+  // Settings retain caller RLS inside this same transaction. Restore the
+  // internal role/context before merging backend-owned progress fields.
+  const saved = (await tx.execute<{ role: string, claims: string | null, sub: string | null, headers: string | null }>(sql`
+    SELECT current_user AS role,
+      pg_catalog.current_setting('request.jwt.claims', true) AS claims,
+      pg_catalog.current_setting('request.jwt.claim.sub', true) AS sub,
+      pg_catalog.current_setting('request.headers', true) AS headers
+  `)).rows[0]
+  const jwt = auth?.authType === 'jwt'
+  await tx.execute(sql`SELECT
+    pg_catalog.set_config('request.jwt.claims', ${JSON.stringify(jwt ? { ...auth.claims, sub: userId, role: 'authenticated' } : { role: 'anon' })}, true),
+    pg_catalog.set_config('request.jwt.claim.sub', ${jwt ? userId : ''}, true),
+    pg_catalog.set_config('request.headers', ${JSON.stringify(jwt ? {} : { capgkey: key })}, true)
+  `)
+  await tx.execute(jwt ? sql`SET LOCAL ROLE authenticated` : sql`SET LOCAL ROLE anon`)
+  const updated = await tx.update(apps).set(settings).where(eq(apps.app_id, appId)).returning({ app_id: apps.app_id })
+  if (!updated[0])
+    throw quickError(401, 'cannot_access_app', 'You can\'t access this app', { app_id: appId })
+  const updatedApp = await tx.execute<AppRow>(sql`
+    SELECT * FROM public.apps WHERE app_id = ${appId}
+  `)
+  const app = updatedApp.rows[0]
+  if (!app)
+    throw quickError(401, 'cannot_access_app', 'You can\'t access this app', { app_id: appId })
+  const completed = current.need_onboarding && !app.need_onboarding
+  await tx.execute(sql`SET LOCAL ROLE ${sql.identifier(saved.role)}`)
+  await tx.execute(sql`SELECT
+    pg_catalog.set_config('request.jwt.claims', ${saved.claims ?? ''}, true),
+    pg_catalog.set_config('request.jwt.claim.sub', ${saved.sub ?? ''}, true),
+    pg_catalog.set_config('request.headers', ${saved.headers ?? ''}, true)
+  `)
+  return { app, completed }
+}
+
+async function completePendingOnboardingRow(tx: OnboardingTx, appId: string) {
+  const completionResult = await tx.execute<AppRow>(sql`
+    UPDATE public.apps
+    SET need_onboarding = false
+    WHERE app_id = ${appId}
+      AND need_onboarding = true
+    RETURNING *
+  `)
+  const app = completionResult.rows[0] as AppRow | undefined
+  return { app, completed: !!app }
+}
+
+async function loadPersistedApp(
+  tx: OnboardingTx,
+  appId: string,
+  app: AppRow | undefined,
+  completed: boolean,
+): Promise<PersistAppOnboardingResult | undefined> {
+  const refreshed = app
+    ? null
+    : await tx.execute<AppRow>(sql`
+        SELECT * FROM public.apps WHERE app_id = ${appId}
+      `)
+  app ??= refreshed?.rows[0] as AppRow | undefined
+  return app ? { app, completed, historyChanges: [] } : undefined
+}
+
+async function mergeOnboardingPatch(
+  tx: OnboardingTx,
+  appId: string,
+  currentOnboarding: unknown,
+  patch: OnboardingPatch,
+  completed: boolean,
+): Promise<PersistAppOnboardingResult> {
+  patch = filterAppOnboardingReportedPatch(currentOnboarding, patch)
+  const mergeResult = await tx.execute<{ onboarding: unknown }>(sql`
+    SELECT public.merge_app_onboarding_setup(
+      ${JSON.stringify(currentOnboarding)}::jsonb,
+      ${JSON.stringify(patch)}::jsonb
+    ) AS onboarding
+  `)
+  if (!mergeResult.rows[0])
+    throw new Error('Cannot merge app onboarding progress')
+  const onboarding = appendAppOnboardingStepHistory(currentOnboarding, mergeResult.rows[0]?.onboarding, patch)
+  const historyChanges = getAppOnboardingStepHistoryChanges(currentOnboarding, onboarding, patch)
+  const result = await tx.execute<AppRow>(sql`
+    UPDATE public.apps
+    SET onboarding = ${JSON.stringify(onboarding)}::jsonb,
+        updated_at = now()
+    WHERE app_id = ${appId}
+    RETURNING *
+  `)
+  const row = result.rows[0] as AppRow | undefined
+  if (!row)
+    throw new Error('App disappeared during onboarding progress update')
+  const completeResult = await tx.execute<{ completed: boolean }>(sql`
+    SELECT public.try_complete_pending_onboarding_if_setup_done(${appId}) AS completed
+  `)
+  completed ||= completeResult.rows[0]?.completed === true
+  const refreshed = await tx.execute<AppRow>(sql`
+    SELECT * FROM public.apps WHERE app_id = ${appId}
+  `)
+  return {
+    app: (refreshed.rows[0] ?? row) as AppRow,
+    completed,
+    historyChanges,
+  }
+}
+
+async function persistAppOnboardingTx(
+  c: Context<MiddlewareKeyVariables>,
+  tx: OnboardingTx,
+  appId: string,
+  patch: OnboardingPatch | undefined,
+  apikey: Database['public']['Tables']['apikeys']['Row'],
+  completePendingOnboarding: boolean,
+  settings?: AppSettings,
+) {
+  const current = await lockAppOnboardingForWrite(tx, appId)
+  if (!current)
+    return undefined
+  await assertOnboardingWritePermission(c, tx, appId, current.owner_org, apikey, settings)
+  let app: AppRow | undefined
+  let completed = false
+  if (settings)
+    ({ app, completed } = await applyAppSettingsInTransaction(c, tx, appId, current, apikey, settings))
+  if (completePendingOnboarding)
+    ({ app, completed } = await completePendingOnboardingRow(tx, appId))
+  if (!patch)
+    return await loadPersistedApp(tx, appId, app, completed)
+  return await mergeOnboardingPatch(tx, appId, current.onboarding, patch, completed)
+}
+
 export async function persistAppOnboarding(
   c: Context<MiddlewareKeyVariables>,
   appId: string,
-  patch: NonNullable<ReturnType<typeof parseAppOnboardingPatch>> | undefined,
+  patch: OnboardingPatch | undefined,
   apikey: Database['public']['Tables']['apikeys']['Row'],
   transactionClient?: PoolClient,
   completePendingOnboarding = false,
@@ -52,118 +229,7 @@
   const pool = transactionClient ? null : getPgClient(c)
   try {
     const drizzle = getDrizzleClient(transactionClient ?? pool!, { logger: false })
-    return await retryAppOnboardingWrite(drizzle, async (tx) => {
-      const current = await lockAppOnboardingForWrite(tx, appId)
-      if (!current)
-        return undefined
-      const auth = c.get('auth')
-      const userId = auth?.userId ?? apikey.user_id
-      const key = auth?.apikey?.key ?? c.get('capgkey') ?? (auth?.authType === 'jwt' ? null : apikey.key)
-      // Recheck under the same lock held by RBAC revocations, not the earlier
-      // request-level permission snapshot.
-      const canUpdateSettings = await checkPermissionPg(c, 'app.update_settings', { appId }, tx, userId, key)
-      if (!canUpdateSettings && (settings
-        || !(await checkPermissionPg(c, 'org.create_app', { orgId: current.owner_org }, tx, userId, key)))) {
-        throw quickError(401, 'cannot_access_app', 'You can\'t access this app', { app_id: appId })
-      }
-      let app: Database['public']['Tables']['apps']['Row'] | undefined
-      let completed = false
-      if (settings) {
-        if (settings.icon_url) {
-          settings = { ...settings, icon_url: resolveWritableImageValue(settings.icon_url, { orgId: current.owner_org, appId }, getStorageAllowedOrigins(c)) ?? undefined }
-          if (!settings.icon_url)
-            throw simpleError('invalid_icon_path', 'Icon path must belong to this app organization')
-        }
-        // Settings retain caller RLS inside this same transaction. Restore the
-        // internal role/context before merging backend-owned progress fields.
-        const saved = (await tx.execute<{ role: string, claims: string | null, sub: string | null, headers: string | null }>(sql`
-          SELECT current_user AS role,
-            pg_catalog.current_setting('request.jwt.claims', true) AS claims,
-            pg_catalog.current_setting('request.jwt.claim.sub', true) AS sub,
-            pg_catalog.current_setting('request.headers', true) AS headers
-        `)).rows[0]
-        const jwt = auth?.authType === 'jwt'
-        await tx.execute(sql`SELECT
-          pg_catalog.set_config('request.jwt.claims', ${JSON.stringify(jwt ? { ...auth.claims, sub: userId, role: 'authenticated' } : { role: 'anon' })}, true),
-          pg_catalog.set_config('request.jwt.claim.sub', ${jwt ? userId : ''}, true),
-          pg_catalog.set_config('request.headers', ${JSON.stringify(jwt ? {} : { capgkey: key })}, true)
-        `)
-        await tx.execute(jwt ? sql`SET LOCAL ROLE authenticated` : sql`SET LOCAL ROLE anon`)
-        const updated = await tx.update(apps).set(settings).where(eq(apps.app_id, appId)).returning({ app_id: apps.app_id })
-        if (!updated[0])
-          throw quickError(401, 'cannot_access_app', 'You can\'t access this app', { app_id: appId })
-        const updatedApp = await tx.execute<Database['public']['Tables']['apps']['Row']>(sql`
-          SELECT * FROM public.apps WHERE app_id = ${appId}
-        `)
-        app = updatedApp.rows[0]
-        if (!app)
-          throw quickError(401, 'cannot_access_app', 'You can\'t access this app', { app_id: appId })
-        completed = current.need_onboarding && !app.need_onboarding
-        await tx.execute(sql`SET LOCAL ROLE ${sql.identifier(saved.role)}`)
-        await tx.execute(sql`SELECT
-          pg_catalog.set_config('request.jwt.claims', ${saved.claims ?? ''}, true),
-          pg_catalog.set_config('request.jwt.claim.sub', ${saved.sub ?? ''}, true),
-          pg_catalog.set_config('request.headers', ${saved.headers ?? ''}, true)
-        `)
-      }
-      if (completePendingOnboarding) {
-        const completionResult = await tx.execute<Database['public']['Tables']['apps']['Row']>(sql`
-          UPDATE public.apps
-          SET need_onboarding = false
-          WHERE app_id = ${appId}
-            AND need_onboarding = true
-          RETURNING *
-        `)
-        app = completionResult.rows[0] as Database['public']['Tables']['apps']['Row'] | undefined
-        completed = !!app
-      }
-
-      if (!patch) {
-        const refreshed = app
-          ? null
-          : await tx.execute<Database['public']['Tables']['apps']['Row']>(sql`
-              SELECT * FROM public.apps WHERE app_id = ${appId}
-            `)
-        app ??= refreshed?.rows[0] as Database['public']['Tables']['apps']['Row'] | undefined
-        return app ? { app, completed, historyChanges: [] } : undefined
-      }
-
-      const currentOnboarding = current.onboarding
-
-      patch = filterAppOnboardingReportedPatch(currentOnboarding, patch)
-      const mergeResult = await tx.execute<{ onboarding: unknown }>(sql`
-        SELECT public.merge_app_onboarding_setup(
-          ${JSON.stringify(currentOnboarding)}::jsonb,
-          ${JSON.stringify(patch)}::jsonb
-        ) AS onboarding
-      `)
-      if (!mergeResult.rows[0])
-        throw new Error('Cannot merge app onboarding progress')
-      const onboarding = appendAppOnboardingStepHistory(currentOnboarding, mergeResult.rows[0]?.onboarding, patch)
-      const historyChanges = getAppOnboardingStepHistoryChanges(currentOnboarding, onboarding, patch)
-      const result = await tx.execute<Database['public']['Tables']['apps']['Row']>(sql`
-        UPDATE public.apps
-        SET onboarding = ${JSON.stringify(onboarding)}::jsonb,
-            updated_at = now()
-        WHERE app_id = ${appId}
-        RETURNING *
-      `)
-      const row = result.rows[0] as Database['public']['Tables']['apps']['Row'] | undefined
-      if (!row)
-        throw new Error('App disappeared during onboarding progress update')
-      const completeResult = await tx.execute<{ completed: boolean }>(sql`
-        SELECT public.try_complete_pending_onboarding_if_setup_done(${appId}) AS completed
-      `)
-      completed ||= completeResult.rows[0]?.completed === true
-      const refreshed = await tx.execute<Database['public']['Tables']['apps']['Row']>(sql`
-        SELECT * FROM public.apps WHERE app_id = ${appId}
-      `)
-      return {
-        app: (refreshed.rows[0] ?? row) as Database['public']['Tables']['apps']['Row'],
-        completed,
-        historyChanges,
-      }
-    })
+    return await retryAppOnboardingWrite(drizzle, tx => persistAppOnboardingTx(c, tx, appId, patch, apikey, completePendingOnboarding, settings))
   }
   finally {
     if (pool)

[Gabriel-R285]

Hi! I’d like to work on this issue.

I’ll split the function into focused helpers following the responsibilities described in the issue, while preserving the existing lock order, RBAC checks, RLS handling, transaction flow, and onboarding behavior. I’ll also run the relevant tests to verify that the refactor doesn’t introduce regressions.

I’ll keep the scope strictly limited to issue #3376.