fix(search): recover GitHub installation setup and verify ownership

Author: Sg312

apps/docs/content/docs/platform/self-hosting/integrations-oauth.mdx

@@ -163,11 +163,14 @@ Expand **Permissions → Repository permissions**. Set **Contents → Access: Re
 
 Expand **Account permissions** and set **Email addresses → Access: Read-only**.
 
+Expand **Organization permissions** and set **Members → Access: Read-only**. Sim uses this to verify that the person connecting an organization installation is an organization owner. Existing installations must approve this permission update; repository read access alone cannot verify ownership.
+
 <Image className="mx-auto h-auto w-full max-w-md" src="/static/search/github-app-email-permission.jpg" alt="GitHub account permissions with only Email addresses selected for Read-only access" width={768} height={929} />
 
 | Permission area | Permission | Access |
 |---|---|---|
 | Repository | Contents | Read-only |
+| Organization | Members | Read-only |
 | Repository | Metadata | Read-only |
 | Account | Email addresses | Read-only |
 

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx

@@ -828,6 +828,8 @@ export function AddConnectorModal({
                       connecting={githubSetup.pending}
                       connectionError={githubSetup.error}
                       onCancel={githubSetup.cancel}
+                      onCheckConnection={() => void githubSetup.checkConnection()}
+                      isChecking={githubSetup.isChecking}
                       onChange={(credentialId) => {
                         if (credentialId !== installationCredentialId)
                           handleFieldChange('repository', '')

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/connector-settings-fields.tsx

@@ -300,6 +300,8 @@ export function ConnectorSettingsFields({
               : undefined
           }
           onCancel={githubSetup.cancel}
+          onCheckConnection={() => void githubSetup.checkConnection()}
+          isChecking={githubSetup.isChecking}
           onChange={onContentCredentialChange}
         >
           {(accessDirty || canReenableMemberSync) && (

apps/sim/connectors/github/installation-connection-field.tsx

@@ -18,6 +18,8 @@ interface GitHubInstallationConnectionFieldProps {
   connectionError?: string | null
   hint?: string
   onCancel?: () => void
+  onCheckConnection: () => void
+  isChecking?: boolean
   children?: ReactNode
 }
 
@@ -35,13 +37,17 @@ export function GitHubInstallationConnectionField({
   connectionError,
   hint,
   onCancel,
+  onCheckConnection,
+  isChecking = false,
   children,
 }: GitHubInstallationConnectionFieldProps) {
   return (
     <ChipModalField type='custom' title='GitHub' hint={hint}>
       {connecting ? (
         <div className='flex items-center gap-2'>
-          <Chip disabled>Connecting GitHub…</Chip>
+          <Chip disabled={isChecking} onClick={onCheckConnection}>
+            {isChecking ? 'Checking GitHub…' : 'Check connection'}
+          </Chip>
           <Chip aria-label='Cancel GitHub connection' onClick={onCancel}>
             Cancel
           </Chip>

apps/sim/hooks/use-github-installation-setup-cache.test.tsx

@@ -34,6 +34,7 @@ describe('GitHub setup credential cache reconciliation', () => {
   let browsing: ReturnType<typeof useOAuthCredentials>
   let unrelated: ReturnType<typeof useOAuthCredentials>
   let completed: boolean
+  let completeOnRecheck: boolean
   let channels: Array<{ onmessage: ((event: MessageEvent<unknown>) => void) | null }>
 
   function Probe() {
@@ -73,6 +74,7 @@ describe('GitHub setup credential cache reconciliation', () => {
     mocks.request.mockReset()
     mocks.connected.mockReset()
     completed = false
+    completeOnRecheck = false
     channels = []
     vi.stubGlobal(
       'BroadcastChannel',
@@ -109,8 +111,15 @@ describe('GitHub setup credential cache reconciliation', () => {
           ],
         }
       }
-      if (contract === startGitHubSearchSetupContract)
-        return { success: true, url: 'https://github.com/apps/sim/installations/new' }
+      if (contract === startGitHubSearchSetupContract) {
+        if (completeOnRecheck) completed = true
+        return {
+          success: true,
+          url: completed
+            ? `${window.location.origin}/credential-groups/complete`
+            : 'https://github.com/apps/sim/installations/new',
+        }
+      }
       if (contract === readGitHubSearchSetupContract)
         return {
           success: true,
@@ -135,6 +144,28 @@ describe('GitHub setup credential cache reconciliation', () => {
     vi.unstubAllGlobals()
   })
 
+  it('unlocks the account selection after rechecking GitHub without a callback or broadcast', async () => {
+    await act(async () =>
+      root.render(
+        <QueryClientProvider client={client}>
+          <Probe />
+        </QueryClientProvider>
+      )
+    )
+    await flush()
+    await act(async () => setup.connect())
+    await flush()
+    completeOnRecheck = true
+    await act(async () => window.dispatchEvent(new Event('focus')))
+    await flush()
+    await flush()
+    expect(mocks.connected).toHaveBeenCalledExactlyOnceWith('installation-new')
+    expect(setup.pending).toBe(false)
+    expect(setup.error).toBeNull()
+    expect(indexing.data?.map(({ id }) => id)).toContain('installation-new')
+    expect(unrelated.data?.map(({ id }) => id)).toEqual(['org-2-existing'])
+  })
+
   it('refreshes mounted Add and Settings lists on completion without a visibility event', async () => {
     await act(async () =>
       root.render(

apps/sim/hooks/use-github-installation-setup.test.tsx

@@ -119,6 +119,51 @@ describe('GitHub installation setup handoff', () => {
     expect(mocks.start).toHaveBeenCalledOnce()
     expect(tab.focus).toHaveBeenCalledOnce()
   })
+  it('rechecks the same attempt on return to Sim without relying on a GitHub redirect', async () => {
+    await act(async () => current.connect())
+    mocks.start.mockResolvedValue({ url: `${window.location.origin}/credential-groups/complete` })
+    await act(async () => window.dispatchEvent(new Event('focus')))
+    expect(mocks.start).toHaveBeenLastCalledWith(mocks.start.mock.calls[0][0])
+    expect(mocks.refetch).toHaveBeenCalledOnce()
+    expect(mocks.connected).not.toHaveBeenCalled()
+    mocks.status = {
+      status: 'completed',
+      credential: { id: 'installation-1', displayName: 'Acme' },
+    }
+    act(() => root.render(<Probe />))
+    expect(current.pending).toBe(false)
+    expect(mocks.connected).toHaveBeenCalledExactlyOnceWith('installation-1')
+  })
+  it('leaves GitHub in place if installation is unfinished and exposes a retry', async () => {
+    await act(async () => current.connect())
+    const initialUrl = tab.location.href
+    await act(async () => current.checkConnection())
+    expect(tab.location.href).toBe(initialUrl)
+    expect(current.error).toContain('Finish setup on GitHub')
+    expect(current.pending).toBe(true)
+    expect(mocks.cancel).not.toHaveBeenCalled()
+  })
+  it('ignores an installation recheck that finishes after cancellation', async () => {
+    await act(async () => current.connect())
+    let resolve!: (value: { url: string }) => void
+    mocks.start.mockReturnValue(
+      new Promise((done) => {
+        resolve = done
+      })
+    )
+    let checking!: Promise<void>
+    act(() => {
+      checking = current.checkConnection()
+    })
+    act(() => current.cancel())
+    await act(async () => {
+      resolve({ url: `${window.location.origin}/credential-groups/complete` })
+      await checking
+    })
+    expect(mocks.refetch).not.toHaveBeenCalled()
+    expect(mocks.connected).not.toHaveBeenCalled()
+    expect(current.pending).toBe(false)
+  })
 
   it('cancels without navigating a late start response or accepting its completion', async () => {
     let resolve!: (value: { url: string }) => void

apps/sim/hooks/use-github-installation-setup.ts

@@ -31,6 +31,7 @@ export function useGitHubInstallationSetup({
   onConnected,
 }: GitHubInstallationSetupProps) {
   const active = useRef<{ setupId: string; tab: Window } | null>(null)
+  const checking = useRef<string | null>(null)
   const callback = useRef(onConnected)
   const [setupId, setSetupId] = useState<string>()
   const [error, setError] = useState<string | null>(null)
@@ -92,6 +93,7 @@ export function useGitHubInstallationSetup({
       active.current.tab.close()
       active.current = null
       setSetupId(undefined)
+      setError(null)
       for (const purpose of [undefined, 'browsing'] as const) {
         void client.invalidateQueries({
           queryKey: oauthCredentialKeys.list(
@@ -124,6 +126,54 @@ export function useGitHubInstallationSetup({
     }
   }, [query.data, query.error, setupId, organizationId, client, cancelSetup])
 
+  const checkConnection = useCallback(
+    async (manual = true) => {
+      const attempt = active.current
+      if (!attempt || !organizationId || !setupId || checking.current === attempt.setupId) return
+      let reopened = false
+      if (manual && attempt.tab.closed) {
+        const tab = window.open('about:blank', '_blank', 'width=600,height=700')
+        if (!tab) {
+          setError('Allow pop-ups for this site to continue GitHub setup, then try again.')
+          return
+        }
+        tab.opener = null
+        attempt.tab = tab
+        reopened = true
+      }
+      checking.current = attempt.setupId
+      setError(null)
+      try {
+        const result = await start({ organizationId, setupId: attempt.setupId })
+        if (active.current !== attempt) return
+        const url = resolveGitHubSetupUrl(result.url, window.location.origin)
+        if (new URL(url).origin === window.location.origin) {
+          if (!attempt.tab.closed) {
+            attempt.tab.location.href = url
+            if (manual) attempt.tab.focus()
+          }
+        } else if (manual) {
+          if (reopened) attempt.tab.location.href = url
+          setError('Finish setup on GitHub, then check the connection again.')
+        }
+        await refetch()
+      } catch (failure) {
+        if (active.current === attempt && manual)
+          setError(getErrorMessage(failure, 'Could not check GitHub. Try again.'))
+      } finally {
+        if (checking.current === attempt.setupId) checking.current = null
+      }
+    },
+    [organizationId, setupId, start, refetch]
+  )
+
+  useEffect(() => {
+    if (!setupId) return
+    const onFocus = () => void checkConnection(false)
+    window.addEventListener('focus', onFocus)
+    return () => window.removeEventListener('focus', onFocus)
+  }, [setupId, checkConnection])
+
   const connect = useCallback(
     async (intent?: StartGitHubSearchSetupBody['intent']) => {
       if (!organizationId) return
@@ -168,5 +218,12 @@ export function useGitHubInstallationSetup({
     void cancelSetup({ organizationId, setupId: attempt.setupId }).catch(() => undefined)
   }, [organizationId, cancelSetup])
 
-  return { connect, cancel, pending: isStarting || Boolean(setupId), error }
+  return {
+    connect,
+    cancel,
+    checkConnection,
+    isChecking: isStarting,
+    pending: isStarting || Boolean(setupId),
+    error,
+  }
 }

apps/sim/lib/knowledge/application/github-installations.test.ts

@@ -36,7 +36,8 @@ vi.mock('@/lib/oauth/github-installation', () => ({
   GitHubInstallationError: class extends Error {
     constructor(
       message: string,
-      readonly status?: number
+      readonly status?: number,
+      readonly operation?: string
     ) {
       super(message)
     }
@@ -196,6 +197,22 @@ describe('GitHub Search installation application operations', () => {
     m.list.mockRejectedValueOnce(error)
     await expect(listGitHubSearchInstallations.execute({ principal, input })).rejects.toBe(error)
   })
+  it('reports missing app membership access without requesting another OAuth connection', async () => {
+    setupReader()
+    m.list.mockRejectedValueOnce(
+      new GitHubInstallationError(
+        'Approve Organization Members read-only access',
+        403,
+        'membership-permissions'
+      )
+    )
+    await expect(listGitHubSearchInstallations.execute({ principal, input })).rejects.toMatchObject(
+      {
+        code: 'validation',
+        message: 'Approve Organization Members read-only access',
+      }
+    )
+  })
   it('preserves transient token refresh failures instead of requesting OAuth', async () => {
     setupReader()
     const error = new ManagedOAuthCredentialError(

apps/sim/lib/knowledge/application/github-installations.ts

@@ -121,6 +121,11 @@ export const listGitHubSearchInstallations = defineAuthorizedKnowledgeUseCase({
           { signal: input.signal }
         )
       } catch (error) {
+        if (
+          error instanceof GitHubInstallationError &&
+          error.operation === 'membership-permissions'
+        )
+          throw new OrchestrationError('validation', error.message)
         /** Only reader-token discovery can request reauthorization; App JWT failures stay errors. */
         if (
           (error instanceof GitHubInstallationError && error.status === 401) ||

apps/sim/lib/knowledge/application/github-setup.test.ts

@@ -307,6 +307,62 @@ describe('GitHub setup lifecycle', () => {
     expect(m.list).not.toHaveBeenCalled()
     expect(m.reader).not.toHaveBeenCalled()
   })
+  it('rechecks an installation left on GitHub settings and completes the original attempt', async () => {
+    const state = new URL((await start()).url).searchParams.get('state')!
+    m.list.mockResolvedValue({
+      available: true,
+      needsUserConnection: false,
+      installations: [installation],
+    })
+    expect(new URL((await start()).url).pathname).toBe('/credential-groups/complete')
+    await expect(status()).resolves.toEqual({ status: 'completed', credential: resultCredential })
+    expect(m.connect).toHaveBeenCalledExactlyOnceWith(
+      expect.objectContaining({
+        principal,
+        input: expect.objectContaining({
+          organizationId: input.organizationId,
+          installationId: '42',
+        }),
+      })
+    )
+    await callback(state)
+    expect(m.connect).toHaveBeenCalledOnce()
+  })
+  it('keeps incomplete installation checks pending without replacing callback state', async () => {
+    const initial = await start()
+    const attempt = await readGitHubSetupAttempt(scope)
+    await expect(start()).resolves.toEqual(initial)
+    expect(await readGitHubSetupAttempt(scope)).toEqual(attempt)
+    expect(m.connect).not.toHaveBeenCalled()
+  })
+  it('lets the user choose after explicitly adding another organization without a callback', async () => {
+    m.list.mockResolvedValue({
+      available: true,
+      needsUserConnection: false,
+      installations: [installation],
+    })
+    const state = new URL((await start('install')).url).searchParams.get('state')!
+    m.list.mockResolvedValue({
+      available: true,
+      needsUserConnection: false,
+      installations: [installation, secondInstallation],
+    })
+    expect(new URL((await start()).url).pathname).toBe('/knowledge/github/setup')
+    await expect(status()).resolves.toEqual({
+      status: 'choosing',
+      installations: [installation, secondInstallation],
+    })
+    expect(m.connect).not.toHaveBeenCalled()
+    expect(new URL((await callback(state)).url).pathname).toBe('/knowledge/github/setup')
+    expect(m.connect).not.toHaveBeenCalled()
+  })
+  it('does not replace an installation attempt when a connection check has a transient failure', async () => {
+    await start()
+    m.list.mockRejectedValueOnce(new Error('GitHub unavailable'))
+    await expect(start()).rejects.toThrow('GitHub unavailable')
+    await expect(status()).resolves.toEqual({ status: 'pending' })
+    expect(m.connect).not.toHaveBeenCalled()
+  })
   it.each([{ userId: 'other-user' }, { sessionId: 'other-browser' }])(
     'cannot read or consume another initiating session: %s',
     async (change) => {
@@ -502,6 +558,26 @@ function deferred<T>() {
 }
 
 describe('GitHub setup atomic claims', () => {
+  it('cancellation during an installation recheck prevents a late grant', async () => {
+    await start()
+    const entered = deferred<void>()
+    const listed = deferred<{
+      available: boolean
+      needsUserConnection: boolean
+      installations: (typeof installation)[]
+    }>()
+    m.list.mockImplementationOnce(() => {
+      entered.resolve()
+      return listed.promise
+    })
+    const pending = start()
+    await entered.promise
+    await cancel()
+    listed.resolve({ available: true, needsUserConnection: false, installations: [installation] })
+    await expect(pending).rejects.toMatchObject({ code: 'conflict' })
+    expect(m.connect).not.toHaveBeenCalled()
+    await expect(status()).resolves.toEqual({ status: 'expired' })
+  })
   it('never wraps nested authorized operations in a global-pool transaction', async () => {
     m.list.mockImplementationOnce(async () => {
       expect(db.transaction).not.toHaveBeenCalled()