fix(credentials): make disconnect idempotent

Author: TheodoreSpeaks

apps/sim/lib/credentials/__tests__/webhook-deactivation.test.ts

@@ -53,18 +53,19 @@ describe('deleteConnectionCredential', () => {
   it('deletes exactly one credential within its canonical workspace scope', async () => {
     dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'credential-1' }])
 
-    await deleteConnectionCredential({
+    const deleted = await deleteConnectionCredential({
       credentialId: 'credential-1',
       workspaceId: 'workspace-1',
       reason: 'user_delete',
     })
 
+    expect(deleted).toBe(true)
     expect(dbChainMockFns.delete).toHaveBeenCalledWith(schemaMock.credential)
     expect(drizzleOrmMock.eq).toHaveBeenCalledWith(schemaMock.credential.id, 'credential-1')
     expect(drizzleOrmMock.eq).toHaveBeenCalledWith(schemaMock.credential.workspaceId, 'workspace-1')
   })
 
-  it('fails fast if the authorized credential disappears before deletion commits', async () => {
+  it('returns an idempotent no-op if a concurrent disconnect wins the delete', async () => {
     dbChainMockFns.returning.mockResolvedValueOnce([])
 
     await expect(
@@ -73,6 +74,6 @@ describe('deleteConnectionCredential', () => {
         workspaceId: 'workspace-1',
         reason: 'user_delete',
       })
-    ).rejects.toThrow('Credential disappeared during deletion')
+    ).resolves.toBe(false)
   })
 })

apps/sim/lib/credentials/application/service-account.test.ts

@@ -92,6 +92,7 @@ describe('credential service-account application operations', () => {
       auditMetadata: { tenantId: 'tenant-1' },
     })
     mocks.listCatalog.mockResolvedValue([{ providerId: 'zoom-service-account' }])
+    mocks.delete.mockResolvedValue(true)
     mocks.requireProvider.mockReturnValue({
       type: 'service_account',
       providerId: 'zoom-service-account',
@@ -199,11 +200,22 @@ describe('credential service-account application operations', () => {
       input: { workspaceId: WORKSPACE_ID, credentialId: credential.id },
     })
 
-    expect(result).toEqual({ credential })
+    expect(result).toEqual({ credential, deleted: true })
     expect(mocks.delete).toHaveBeenCalledWith({
       credentialId: credential.id,
       workspaceId: WORKSPACE_ID,
       reason: 'user_delete',
     })
   })
+
+  it('treats a concurrent disconnect as an idempotent success', async () => {
+    mocks.delete.mockResolvedValue(false)
+
+    const result = await deleteCredentialUseCase.execute({
+      principal,
+      input: { workspaceId: WORKSPACE_ID, credentialId: credential.id },
+    })
+
+    expect(result).toEqual({ credential, deleted: false })
+  })
 })

apps/sim/lib/credentials/application/service-account.ts

@@ -141,6 +141,7 @@ export interface DeleteCredentialInput {
 
 export interface DeleteCredentialResult {
   credential: CredentialRow
+  deleted: boolean
 }
 
 async function resolveCredentialContext(
@@ -176,27 +177,31 @@ export const deleteCredentialUseCase = defineAuthorizedWorkspaceUseCase({
       )
     }
 
-    await deleteConnectionCredential({
+    const deleted = await deleteConnectionCredential({
       credentialId: input.credentialId,
       workspaceId: context.workspaceId,
       reason: 'user_delete',
     })
-    return { credential: context.credential }
+    return { credential: context.credential, deleted }
   },
-  projectAudit: ({ result }) => ({
-    action: AuditAction.CREDENTIAL_DELETED,
-    resourceType: AuditResourceType.CREDENTIAL,
-    resourceId: result.credential.id,
-    resourceName: result.credential.displayName,
-    description: `Deleted ${result.credential.type} credential "${result.credential.displayName}" (user_delete)`,
-    metadata: {
-      reason: 'user_delete',
-      credentialType: result.credential.type,
-      providerId: result.credential.providerId,
-      accountId: result.credential.accountId,
-    },
-  }),
+  projectAudit: ({ result }) =>
+    result.deleted
+      ? {
+          action: AuditAction.CREDENTIAL_DELETED,
+          resourceType: AuditResourceType.CREDENTIAL,
+          resourceId: result.credential.id,
+          resourceName: result.credential.displayName,
+          description: `Deleted ${result.credential.type} credential "${result.credential.displayName}" (user_delete)`,
+          metadata: {
+            reason: 'user_delete',
+            credentialType: result.credential.type,
+            providerId: result.credential.providerId,
+            accountId: result.credential.accountId,
+          },
+        }
+      : [],
   afterSuccess: ({ principal, context, result }) => {
+    if (!result.deleted) return
     captureServerEvent(
       principalUserId(principal),
       'credential_deleted',

apps/sim/lib/credentials/deletion.ts

@@ -80,7 +80,7 @@ export async function deleteCredential(params: DeleteCredentialParams): Promise<
 /** Clears references and deletes one connection without surface audit attribution. */
 export async function deleteConnectionCredential(
   params: DeleteConnectionCredentialParams
-): Promise<void> {
+): Promise<boolean> {
   const { credentialId, workspaceId } = params
   await clearCredentialRefs(credentialId, workspaceId)
   const deleted = await db
@@ -89,13 +89,16 @@ export async function deleteConnectionCredential(
       and(eq(schema.credential.id, credentialId), eq(schema.credential.workspaceId, workspaceId))
     )
     .returning({ id: schema.credential.id })
-  if (deleted.length !== 1) throw new Error('Credential disappeared during deletion')
+  if (deleted.length > 1) throw new Error('Credential deletion affected multiple rows')
 
-  logger.info('Deleted credential', {
-    credentialId,
-    workspaceId,
-    reason: params.reason,
-  })
+  if (deleted.length === 1) {
+    logger.info('Deleted credential', {
+      credentialId,
+      workspaceId,
+      reason: params.reason,
+    })
+  }
+  return deleted.length === 1
 }
 
 /**