fix(gitlab): address setup review and test expectations

Author: icecrasher321

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/use-connector-settings-form.ts

@@ -316,20 +316,28 @@ export function useConnectorSettingsForm({
         !hiddenCapFieldIds.has(key) && !valuesEqual(connector.sourceConfig[key], value)
     )
 
-  const handleSave = () => {
+  const {
+    apiKey: permissionApiKey,
+    complete: permissionsComplete,
+    dirty: permissionsDirty,
+    input: permissionInput,
+    reset: resetPermissions,
+  } = gitlabPermissions
+
+  const handleSave = useCallback(() => {
     if (
       !searchSettingsAllowed ||
       !settingsComplete ||
       accessDirty ||
-      (showGitLabPermissions && !gitlabPermissions.complete)
+      (showGitLabPermissions && !permissionsComplete)
     )
       return
     setError(null)
 
     const updates: UpdateConnectorBody = {}
-    if (showGitLabPermissions && gitlabPermissions.dirty) {
-      updates.permissionConfig = gitlabPermissions.input
-      if (gitlabPermissions.apiKey.trim()) updates.apiKey = gitlabPermissions.apiKey
+    if (showGitLabPermissions && permissionsDirty) {
+      updates.permissionConfig = permissionInput
+      if (permissionApiKey.trim()) updates.apiKey = permissionApiKey
     }
 
     if (syncInterval !== connector.syncIntervalMinutes) {
@@ -338,8 +346,9 @@ export function useConnectorSettingsForm({
 
     const resolved = resolveSourceConfig()
     const changedEntries: Record<string, unknown> = {}
+    const saveHiddenFieldIds = derivedAclCapFieldIds(connectorConfig, access.accessMode)
     for (const [key, value] of Object.entries(resolved)) {
-      if (hiddenCapFieldIds.has(key)) continue
+      if (saveHiddenFieldIds.has(key)) continue
       if (!valuesEqual(connector.sourceConfig[key], value)) changedEntries[key] = value
     }
 
@@ -365,7 +374,7 @@ export function useConnectorSettingsForm({
       { knowledgeBaseId, connectorId: connector.id, updates },
       {
         onSuccess: (updated) => {
-          gitlabPermissions.reset(updated.permissionConfig)
+          resetPermissions(updated.permissionConfig)
           onSaved(updated)
         },
         onError: (err) => {
@@ -374,7 +383,27 @@ export function useConnectorSettingsForm({
         },
       }
     )
-  }
+  }, [
+    access.accessMode,
+    accessDirty,
+    canonicalModes,
+    connector,
+    connectorConfig,
+    knowledgeBaseId,
+    onSaved,
+    permissionApiKey,
+    permissionInput,
+    permissionsComplete,
+    permissionsDirty,
+    persistedCanonicalModes,
+    resetPermissions,
+    resolveSourceConfig,
+    searchSettingsAllowed,
+    settingsComplete,
+    showGitLabPermissions,
+    syncInterval,
+    updateConnector,
+  ])
 
   /**
    * The mode switch is its own admin operation: it rewrites document access

apps/sim/app/workspace/[workspaceId]/search/components/search-source-setup.test.tsx

@@ -1026,9 +1026,11 @@ describe('Search source setup with real connector dialogs', () => {
     )
   })
 
-  it('opens GitLab with its single central method and submits the custom host and PAT', async () => {
+  it('opens GitLab token tabs and submits the custom host and PAT with managed access', async () => {
     await render(setup(), '?addConnector=gitlab')
-    expect(document.body.textContent).toContain('Admin or service account')
+    expect(button('Administrator token')).toHaveAttribute('aria-checked', 'true')
+    expect(button('Non-admin token')).toHaveAttribute('aria-checked', 'false')
+    expect(document.body.textContent).not.toContain('Connection method')
     expect(document.body.textContent).not.toContain('Member accounts')
     expect(button('Connect & Sync')).toBeDisabled()
     await fill('Enter your GitLab PAT', 'test-pat')
@@ -1197,14 +1199,16 @@ describe('member content credentials in real add and edit dialogs', () => {
     )
     await click(card!)
     expect(document.body.textContent).not.toContain('Connected members')
-    expect(button('Workspace')).toHaveAttribute('aria-checked', 'true')
+    expect(button('Administrator token')).toHaveAttribute('aria-checked', 'true')
+    expect(document.body.textContent).not.toContain('Connection method')
     await fill('Enter your GitLab PAT', 'new-pat')
+    await fill('gitlab.example.com', 'gitlab.example.test')
     await fill('group/project or numeric ID', '1')
     expect(button('Connect & Sync')).toBeEnabled()
     await click(button('Connect & Sync'))
     expect(mocks.create.mock.calls[1][0]).toMatchObject({
       connectorType: 'gitlab',
-      accessMode: 'workspace',
+      accessMode: 'admin',
       apiKey: 'new-pat',
     })
     expect(mocks.create.mock.calls[1][0].sourceConfig).not.toHaveProperty('excludeChannels')
@@ -1978,8 +1982,9 @@ describe('canonical Search connector safety', () => {
     )
     expect(gitlab).toBeDefined()
     await click(gitlab!)
-    expect(document.body.textContent).toContain('Admin or service account')
-    expect(document.querySelector('[role="radio"][aria-checked="true"]')).toBeNull()
+    expect(button('Administrator token')).toHaveAttribute('aria-checked', 'true')
+    expect(button('Non-admin token')).toHaveAttribute('aria-checked', 'false')
+    expect(document.body.textContent).not.toContain('Connection method')
     expect(
       Array.from(document.querySelectorAll('button')).some(
         (node) => node.textContent === 'Workspace'
@@ -2027,8 +2032,9 @@ describe('resuming Search source setup', () => {
 
   it('reopens the source from the URL even when the source filter hides its row', async () => {
     await render(setup(), '?search=nothing-matches&addConnector=gitlab&credentialDraftId=draft-1')
-    expect(document.body.textContent).toContain('Admin or service account')
-    expect(document.querySelector('[role="radio"][aria-checked="true"]')).toBeNull()
+    expect(button('Administrator token')).toHaveAttribute('aria-checked', 'true')
+    expect(button('Non-admin token')).toHaveAttribute('aria-checked', 'false')
+    expect(document.body.textContent).not.toContain('Connection method')
     expect(document.body.textContent).toContain('Add GitLab project')
     expect(document.body.textContent).not.toContain('Sync Frequency')
     expect(document.body.textContent).not.toContain('Sync automatically')

apps/sim/connectors/gitlab/permission-config/parser.test.ts

@@ -80,13 +80,11 @@ describe('GitLab CSV permission uploads', () => {
       )
     ).toThrow('4 MiB')
   })
-  it('accepts 100,000 rows within a bounded parsing budget and rejects the next row', () => {
+  it('accepts 100,000 rows and rejects the next row', () => {
     const content = Array.from({ length: 100_000 }, (_, i) => `${i + 1},u${i}@example.com`).join(
       '\n'
     )
-    const start = performance.now()
     expect(parseGitLabCsv({ filename: 'users.csv', content }, 'userMapping')).toHaveLength(100_000)
-    expect(performance.now() - start).toBeLessThan(5000)
     expect(() =>
       parseGitLabCsv(
         { filename: 'users.csv', content: `${content}\n100001,last@example.com` },

apps/sim/connectors/gitlab/permission-config/use-permission-form.ts

@@ -1,6 +1,6 @@
 'use client'
 
-import { useRef, useState } from 'react'
+import { useCallback, useMemo, useRef, useState } from 'react'
 import { getErrorMessage } from '@sim/utils/errors'
 import type {
   GitLabPermissionData,
@@ -41,18 +41,21 @@ export function useGitLabPermissionForm(initial?: GitLabPermissionData) {
     Boolean(apiKey) ||
     (mode === 'csv' &&
       Object.values(files).some((file) => file.upload || file.error || file.loading))
-  const input: GitLabPermissionUploadInput = {
-    provider: 'gitlab',
-    mode,
-    ...(saved ? { expectedRevision: saved.revision } : {}),
-    ...(mode === 'csv'
-      ? {
-          userMapping: files.userMapping?.upload,
-          projectPermissions: files.projectPermissions?.upload,
-        }
-      : {}),
-  }
-  async function selectFile(kind: GitLabCsvKind, file: File) {
+  const input = useMemo<GitLabPermissionUploadInput>(
+    () => ({
+      provider: 'gitlab',
+      mode,
+      ...(saved ? { expectedRevision: saved.revision } : {}),
+      ...(mode === 'csv'
+        ? {
+            userMapping: files.userMapping?.upload,
+            projectPermissions: files.projectPermissions?.upload,
+          }
+        : {}),
+    }),
+    [files, mode, saved]
+  )
+  const selectFile = useCallback(async (kind: GitLabCsvKind, file: File) => {
     const generation = ++generations.current[kind]
     setFiles((previous) => ({ ...previous, [kind]: { loading: true } }))
     let selected: SelectedFile
@@ -69,15 +72,15 @@ export function useGitLabPermissionForm(initial?: GitLabPermissionData) {
     }
     if (generation === generations.current[kind])
       setFiles((previous) => ({ ...previous, [kind]: selected }))
-  }
-  function reset(next?: GitLabPermissionData) {
+  }, [])
+  const reset = useCallback((next?: GitLabPermissionData) => {
     generations.current.userMapping++
     generations.current.projectPermissions++
     setFiles({})
     setApiKey('')
     setSaved(next)
     setMode(next?.mode ?? 'administrator')
-  }
+  }, [])
   return {
     mode,
     setMode,

apps/sim/lib/knowledge/orchestration/connectors.test.ts

@@ -430,6 +430,52 @@ describe('performUpdateKnowledgeConnector', () => {
     expect(dbChainMockFns.select).not.toHaveBeenCalled()
   })
 
+  it.each(['permissions', 'token'] as const)(
+    'commits a %s-only change without dispatching a content sync',
+    async (change) => {
+      const existing = {
+        id: 'conn-1',
+        connectorType: 'gitlab',
+        accessMode: 'admin',
+        status: 'active',
+        updatedAt: new Date('2026-01-01T00:00:00Z'),
+      }
+      dbChainMockFns.limit.mockResolvedValueOnce([existing])
+      dbChainMockFns.returning.mockResolvedValueOnce([existing])
+      const write = vi.fn().mockResolvedValue(undefined)
+      const outcome = await performUpdateKnowledgeConnector({
+        ...ACTOR,
+        knowledgeBase: KB,
+        connectorId: existing.id,
+        updates: {},
+        permissionChange: {
+          requiresAclReset: false,
+          requiresContentSync: false,
+          ...(change === 'token' ? { encryptedApiKey: 'encrypted-fixture-pat' } : {}),
+          populateSyncContext: vi.fn(),
+          write,
+        },
+        resolveBillingAttribution,
+      })
+
+      expect(outcome).toMatchObject({ success: true })
+      expect(write).toHaveBeenCalledWith(expect.anything(), existing.id)
+      expect(mockRecordAudit).toHaveBeenCalledWith(
+        expect.objectContaining({
+          metadata: expect.objectContaining({ updatedFields: ['permissionConfig'] }),
+        })
+      )
+      expect(mockDispatchSync).not.toHaveBeenCalled()
+      expect(mockDispatchMemberSync).not.toHaveBeenCalled()
+      expect(resolveBillingAttribution).not.toHaveBeenCalled()
+      if (change === 'token') {
+        expect(dbChainMockFns.set).toHaveBeenCalledWith(
+          expect.objectContaining({ encryptedApiKey: 'encrypted-fixture-pat' })
+        )
+      }
+    }
+  )
+
   it('classifies a sub-hourly interval on an unentitled workspace as forbidden', async () => {
     dbChainMockFns.limit.mockResolvedValueOnce([
       { id: 'conn-1', connectorType: 'notion', accessMode: 'workspace' },

apps/sim/lib/knowledge/orchestration/connectors.ts

@@ -722,13 +722,9 @@ export async function performUpdateKnowledgeConnector(
   const updatedFields = Object.keys(updates).filter(
     (key) => updates[key as keyof typeof updates] !== undefined
   )
-  const changedFields: string[] = updatedFields
-  if (params.permissionChange) changedFields.push('permissionConfig')
+  if (params.permissionChange) updatedFields.push('permissionConfig')
   if (updatedFields.length === 0) {
-    return fail(
-      'At least one of sourceConfig, syncIntervalMinutes, or status is required',
-      'validation'
-    )
+    return fail('At least one connector setting or permission change is required', 'validation')
   }
 
   const existing = await getKnowledgeConnector(kb.id, connectorId)