fix(copilot): coherent service-account rejection in oauth_get_auth_link

Review round on #5786: - The service-account-id guard threw into the generic catch, which overwrote its recovery hint with a "connect manually" message and a workspace oauth_url — contradictory signals. It now returns a coherent failure directly, before the try, with no oauth_url. - Normalize spaces/underscores before the check so a readable form ("slack custom bot", "google service account") is caught too, not passed to the fuzzy OAuth resolver. - Remove listServiceAccountIntegrationNames — dead after the tool was removed (its only caller was the deleted handler's error copy).
simstudioai · TheodoreSpeaks · Jul 22, 2026 · Jul 20, 2026 · Jul 20, 2026 · Jul 20, 2026
commit 1d97ff0e5ceb9b5f6dde28229579cdf92cc36855
diff --git a/apps/sim/lib/copilot/tools/handlers/oauth.test.ts b/apps/sim/lib/copilot/tools/handlers/oauth.test.ts
@@ -224,18 +224,20 @@ describe('executeOAuthGetAuthLink service account rejection', () => {
    * shared bot. Failing loudly is the point: a wrong link that looks right is
    * worse than an error the agent can recover from.
    */
-  it('rejects a service account id instead of degrading to the OAuth flow', async () => {
+  it('rejects a service account id with a coherent recovery message, not a workspace link', async () => {
     const result = await executeOAuthGetAuthLink({ providerName: 'slack-custom-bot' }, context)
 
     expect(result.success).toBe(false)
     expect(result.error).toContain('service account')
     expect(result.error).toContain('service_account credential tag')
-    // The failure output must not carry an authorize URL the agent could
-    // surface as a Connect button anyway.
-    expect((result.output as { setup_url?: string }).setup_url).toBeUndefined()
-    expect((result.output as { oauth_url: string }).oauth_url).not.toContain(
-      '/api/auth/oauth2/authorize'
-    )
+    const output = result.output as { setup_url?: string; oauth_url?: string; message: string }
+    // The rejection must not fall into the generic catch, which would attach a
+    // contradicting workspace oauth_url and a "connect manually" message — the
+    // agent would then surface a workspace link instead of the tag.
+    expect(output.setup_url).toBeUndefined()
+    expect(output.oauth_url).toBeUndefined()
+    expect(output.message).toContain('service_account credential tag')
+    expect(output.message).not.toContain('Connect manually')
   })
 
   it.each([
@@ -244,6 +246,11 @@ describe('executeOAuthGetAuthLink service account rejection', () => {
     'google-service-account',
     'atlassian-service-account',
     'SLACK-CUSTOM-BOT',
+    // Readable forms must be normalized (spaces/underscores → hyphens) so they
+    // are caught too, not passed to the fuzzy OAuth resolver.
+    'slack custom bot',
+    'google service account',
+    'notion_service_account',
   ])('rejects %s', async (providerName) => {
     const result = await executeOAuthGetAuthLink({ providerName }, context)
     expect(result.success).toBe(false)

diff --git a/apps/sim/lib/copilot/tools/handlers/oauth.ts b/apps/sim/lib/copilot/tools/handlers/oauth.ts
@@ -15,6 +15,25 @@ export async function executeOAuthGetAuthLink(
   const rawCredentialId = rawParams.credentialId || rawParams.credential_id
   const credentialId = rawCredentialId ? String(rawCredentialId) : undefined
   const baseUrl = getBaseUrl()
+
+  // A service account is not an OAuth provider. Catch it here — before the fuzzy
+  // resolver's substring match can swallow e.g. `slack-custom-bot` into the
+  // Slack OAuth service — and return a coherent failure directly rather than
+  // throwing into the generic catch below, which would attach a contradicting
+  // workspace `oauth_url` and "connect manually" message. Normalize spaces and
+  // underscores so a readable form ("slack custom bot") is caught too.
+  const serviceAccountId = providerName
+    .toLowerCase()
+    .trim()
+    .replace(/[\s_]+/g, '-')
+  if (isServiceAccountProviderId(serviceAccountId)) {
+    const message =
+      `"${providerName}" is a service account, not an OAuth provider. ` +
+      `Emit a service_account credential tag with the service's OAuth provider ` +
+      `value instead (e.g. "slack") — it opens the service account setup form in chat.`
+    return { success: false, error: message, output: { message } }
+  }
+
   try {
     if (!context.workspaceId || !context.userId) {
       throw new Error('workspaceId and userId are required to generate an OAuth link')
@@ -107,19 +126,6 @@ async function generateOAuthLink(
   const allServices = getAllOAuthServices()
   const normalizedInput = providerName.toLowerCase().trim()
 
-  // Reject a service-account id before the fuzzy pass below can swallow it.
-  // That pass matches on substring containment, so `slack-custom-bot` contains
-  // `slack` and silently resolves to the Slack OAuth service — the tool then
-  // returns a personal-OAuth authorize URL, reports success, and the user
-  // connects their own account when they asked for a shared bot.
-  if (isServiceAccountProviderId(normalizedInput)) {
-    throw new Error(
-      `"${providerName}" is a service account, not an OAuth provider. ` +
-        `Emit a service_account credential tag with the service's OAuth provider ` +
-        `value instead (e.g. "slack") — it opens the service account setup form in chat.`
-    )
-  }
-
   const matched =
     allServices.find((s) => s.providerId === normalizedInput) ||
     allServices.find((s) => s.name.toLowerCase() === normalizedInput) ||

diff --git a/apps/sim/lib/integrations/oauth-service.test.ts b/apps/sim/lib/integrations/oauth-service.test.ts
@@ -4,7 +4,6 @@
 import { describe, expect, it } from 'vitest'
 import integrationsJson from '@/lib/integrations/integrations.json'
 import {
-  listServiceAccountIntegrationNames,
   resolveOAuthServiceForSlug,
   resolveServiceAccountIntegration,
 } from '@/lib/integrations/oauth-service'
@@ -169,13 +168,4 @@ describe('resolveServiceAccountIntegration', () => {
     expect(resolveServiceAccountIntegration('')).toBeNull()
     expect(resolveServiceAccountIntegration('   ')).toBeNull()
   })
-
-  it.concurrent('reports a service-account provider for every match it returns', () => {
-    const names = listServiceAccountIntegrationNames()
-    expect(names.length).toBeGreaterThan(0)
-    for (const name of names) {
-      const match = resolveServiceAccountIntegration(name)
-      expect(match?.serviceAccountProviderId).toBeTruthy()
-    }
-  })
 })
diff --git a/apps/sim/lib/integrations/oauth-service.ts b/apps/sim/lib/integrations/oauth-service.ts
@@ -152,11 +152,3 @@ export function resolveServiceAccountIntegration(
     null
   )
 }
-
-/**
- * Names every integration that offers a service-account flow, for error copy
- * that has to tell the agent what it could have asked for instead.
- */
-export function listServiceAccountIntegrationNames(): string[] {
-  return SERVICE_ACCOUNT_INTEGRATIONS.map((entry) => entry.serviceName)
-}