From 3489b81dd8a4c46159bfeed8f60f4c5f0b65d853 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Wed, 16 Sep 2026 17:59:35 -0700 Subject: [PATCH 1/3] fix(credential-groups): expose provider emails and align people rows --- .../docs/workflows/blocks/credential.mdx | 16 ++- apps/sim/blocks/blocks/credential.test.ts | 44 +++++++ apps/sim/blocks/blocks/credential.ts | 50 +++++--- .../organization-account-people.test.tsx | 52 ++++++++ .../organization-account-people.tsx | 90 +++++++------ .../organization-person-connections.tsx | 99 ++++++++------- .../credential/credential-handler.test.ts | 81 +++++++++++- .../handlers/credential/credential-handler.ts | 7 +- .../application/list-credentials.test.ts | 30 +++++ .../application/list-credentials.ts | 4 +- .../lib/credential-groups/credentials.test.ts | 120 +++++++++++++++++- apps/sim/lib/credential-groups/credentials.ts | 19 ++- .../sim/scripts/block-registry-source.test.ts | 36 ++++++ apps/sim/scripts/block-registry-source.ts | 51 ++++++++ apps/sim/scripts/check-block-registry.ts | 58 +-------- 15 files changed, 591 insertions(+), 166 deletions(-) create mode 100644 apps/sim/blocks/blocks/credential.test.ts create mode 100644 apps/sim/scripts/block-registry-source.test.ts create mode 100644 apps/sim/scripts/block-registry-source.ts diff --git a/apps/docs/content/docs/workflows/blocks/credential.mdx b/apps/docs/content/docs/workflows/blocks/credential.mdx index 529c0846acc..951fa1ab00b 100644 --- a/apps/docs/content/docs/workflows/blocks/credential.mdx +++ b/apps/docs/content/docs/workflows/blocks/credential.mdx @@ -81,7 +81,17 @@ Filter the returned OAuth credentials by provider. Select one or more providers An organization owner or admin must first [set up connected accounts](/platform/connected-accounts) and allow this workflow's workspace. The block uses the organization that owns the workspace; there is no credential group or organization selector. -Every authorized workflow in an allowed workspace can use every active contribution in the organization's pool. Results are not restricted to the running user's own accounts, and no separate per-workflow grant is required. Normal workflow permissions still apply. +Every authorized workflow in an allowed workspace can discover active contributions for the integrations allowed in that workspace. Results are not restricted to the running user's own accounts, and no separate per-workflow grant is required. Normal workflow permissions still apply. Workspace and integration access are checked again on every page. + +### Discover accounts by provider + +1. Choose **List Organization Accounts**. +2. Select a provider such as **Gmail** in **Providers**. Leave it empty to list all allowed providers. +3. Leave **Email** blank. You do not need to know an account's email to discover it. +4. Read **emails** for the provider account addresses, or **credentials** for the corresponding account references. +5. While **hasMore** is true, pass **nextCursor** as **Cursor** with the same filters to read the next page. + +Multiple accounts are returned separately, including accounts contributed by the same person. Disconnected accounts, accounts needing reconnection, and revoked invitations are excluded. Listing never chooses an account automatically for a downstream block. ### Inputs @@ -102,7 +112,9 @@ Find operations fail unless there is exactly one active matching connection. Lis **Find Organization Account** returns `credentialId`, `displayName`, `providerId`, and the invitation `email`. Pass `credentialId` into the corresponding integration block's credential field in advanced mode. -**List Organization Accounts** returns these account references in `credentials`, along with `count`, `hasMore`, and `nextCursor`. `count` is the number returned on this page. Feed `credentials` into a ForEach loop and use `` inside the loop. To process additional pages, pass `nextCursor` into another call with the same filters while `hasMore` is true; the block does not fetch all pages automatically. +**List Organization Accounts** returns these account references in `credentials`, with an additional `accountEmail` field containing the email verified by the OAuth provider. The existing `email` field remains the person's invitation address, which can differ from their provider account address. An optional **Email** input continues to filter by that exact invitation address. + +The list also returns `emails`, `count`, `hasMore`, and `nextCursor`. `emails` contains the provider account addresses on this page in the same order as `credentials`; it preserves separate accounts even when addresses repeat. `count` is the number of accounts returned on this page. Feed `credentials` into a ForEach loop and use `` inside the loop. To process additional pages, pass `nextCursor` into another call with the same filters while `hasMore` is true; the block does not fetch all pages automatically. For example, name a Credential block **account**, choose **Find Organization Account**, set **Email** to `alex@example.com`, and select **Gmail**. Reference `` in a Gmail block to act using Alex's contribution. diff --git a/apps/sim/blocks/blocks/credential.test.ts b/apps/sim/blocks/blocks/credential.test.ts new file mode 100644 index 00000000000..645d1c61fe1 --- /dev/null +++ b/apps/sim/blocks/blocks/credential.test.ts @@ -0,0 +1,44 @@ +/** @vitest-environment node */ +import { createBlock } from '@sim/testing' +import { expect, it, vi } from 'vitest' + +vi.mock('@/triggers', () => ({ getTrigger: () => ({ subBlocks: [] }) })) + +import { CredentialBlock } from '@/blocks/blocks/credential' +import { collectBlockFieldIssues } from '@/serializer/index' + +it.each(['list_organization_accounts', 'list_organization_mcp_connections'])( + 'allows %s through workflow validation without an email', + (operation) => { + const params = { operation, organizationProviders: ['google-email'], mcpProvider: 'fireflies' } + const block = createBlock({ + type: 'credential', + subBlocks: { + operation: { id: 'operation', type: 'dropdown', value: operation }, + organizationProviders: { + id: 'organizationProviders', + type: 'dropdown', + value: ['google-email'], + }, + mcpProvider: { id: 'mcpProvider', type: 'dropdown', value: 'fireflies' }, + }, + }) + expect(collectBlockFieldIssues(block, CredentialBlock, params).missingRequiredFields).toEqual( + [] + ) + } +) + +it('continues to require the enrollment email when finding one organization account', () => { + const block = createBlock({ + type: 'credential', + subBlocks: { + operation: { id: 'operation', type: 'dropdown', value: 'find_organization_account' }, + organizationProvider: { id: 'organizationProvider', type: 'dropdown', value: 'google-email' }, + }, + }) + const params = { operation: 'find_organization_account', organizationProvider: 'google-email' } + expect(collectBlockFieldIssues(block, CredentialBlock, params).missingRequiredFields).toEqual([ + 'Email', + ]) +}) diff --git a/apps/sim/blocks/blocks/credential.ts b/apps/sim/blocks/blocks/credential.ts index 4dd13a27dc1..77b4c3fa782 100644 --- a/apps/sim/blocks/blocks/credential.ts +++ b/apps/sim/blocks/blocks/credential.ts @@ -21,12 +21,15 @@ export const CredentialBlock: BlockConfig = { name: 'Credential', description: 'Select credentials or find organization accounts and MCP connections', longDescription: - 'Select workspace OAuth credentials or find and list organization accounts in an allowlisted workspace. Organization accounts are shared with every authorized workflow in that workspace. Returns credential references and account metadata. Manage invitations in organization settings.', + 'Select workspace OAuth credentials or find and list organization accounts in an allowlisted workspace. List Organization Accounts discovers connected accounts by provider without requiring an email. An optional exact enrollment email narrows the list. Only active accounts for integrations allowed in the executing workspace are returned; disconnected accounts are excluded. Results are paginated using hasMore and nextCursor. Manage invitations in organization settings.', bestPractices: ` - Use "Select Credential" to define an OAuth credential once and reference in multiple downstream blocks instead of repeating credential IDs. - Use "List Credentials" with a ForEach loop to iterate over all OAuth accounts (e.g. all Gmail accounts). - Use the Provider filter to narrow results to specific services (e.g. Gmail, Slack). - - The outputs are credential ID references, not secret values — they are safe to log and inspect. + - Use "List Organization Accounts" with Providers selected and Email blank to discover all accessible accounts for those integrations. + - Organization lists return one page at a time. While hasMore is true, pass nextCursor as Cursor with the same filters to get every matching account. + - "Find Organization Account" requires an exact enrollment email and provider, and fails unless exactly one active account matches. + - Outputs contain account identities and credential references, never secret values. - To switch credentials across environments, replace the single Credential block rather than updating every downstream block. `, docsLink: 'https://docs.sim.ai/workflows/blocks/credential', @@ -39,7 +42,11 @@ export const CredentialBlock: BlockConfig = { select: ['Select an OAuth credential'], list: ['List OAuth credentials', { text: 'for', field: 'providerFilter' }], find_organization_account: ['Find organization account', { text: 'for', field: 'email' }], - list_organization_accounts: ['List organization accounts', { text: 'for', field: 'email' }], + list_organization_accounts: [ + 'List organization accounts', + { text: 'from', field: 'organizationProviders' }, + { text: 'for', field: 'email' }, + ], find_organization_mcp_connection: [ 'Find organization MCP connection', { text: 'for', field: 'email' }, @@ -95,17 +102,6 @@ export const CredentialBlock: BlockConfig = { canonicalParamId: 'credentialId', condition: { field: 'operation', value: 'select' }, }, - { - id: 'email', - title: 'Email', - type: 'short-input', - placeholder: 'person@example.com', - condition: { field: 'operation', value: ORGANIZATION_OPERATIONS }, - required: { - field: 'operation', - value: ['find_organization_account', 'find_organization_mcp_connection'], - }, - }, { id: 'organizationProvider', title: 'Provider', @@ -118,6 +114,8 @@ export const CredentialBlock: BlockConfig = { id: 'organizationProviders', title: 'Providers', type: 'dropdown', + placeholder: 'All allowed providers', + emptyIsValid: true, multiSelect: true, selectorKey: 'workspace.credentialGroupProviders', condition: { field: 'operation', value: 'list_organization_accounts' }, @@ -130,6 +128,17 @@ export const CredentialBlock: BlockConfig = { condition: { field: 'operation', value: MCP_OPERATIONS }, required: { field: 'operation', value: 'find_organization_mcp_connection' }, }, + { + id: 'email', + title: 'Email', + type: 'short-input', + placeholder: 'Optional for lists; exact enrollment email', + condition: { field: 'operation', value: ORGANIZATION_OPERATIONS }, + required: { + field: 'operation', + value: ['find_organization_account', 'find_organization_mcp_connection'], + }, + }, { id: 'limit', title: 'Limit', @@ -152,7 +161,10 @@ export const CredentialBlock: BlockConfig = { }, inputs: { operation: { type: 'string', description: 'Credential operation' }, - email: { type: 'string', description: 'Enrollment email' }, + email: { + type: 'string', + description: 'Exact enrollment email; optional for lists, required for find operations', + }, organizationProvider: { type: 'string', description: 'Organization OAuth provider ID for an exact match', @@ -199,9 +211,15 @@ export const CredentialBlock: BlockConfig = { credentials: { type: 'json', description: - 'Array of OAuth credential objects, each with credentialId, displayName, and providerId', + 'OAuth credential objects with credentialId, displayName, and providerId. Organization accounts also include email (enrollment address), accountEmail (provider account address), providerSubjectId, and providerTenantId.', condition: { field: 'operation', value: ['list', 'list_organization_accounts'] }, }, + emails: { + type: 'json', + description: + 'Provider account email addresses on this page, in the same order as credentials. Multiple accounts are preserved; follow nextCursor while hasMore is true for additional pages.', + condition: { field: 'operation', value: 'list_organization_accounts' }, + }, count: { type: 'number', description: 'Number of connections returned', diff --git a/apps/sim/ee/credential-groups/components/organization-account-people.test.tsx b/apps/sim/ee/credential-groups/components/organization-account-people.test.tsx index 99f2877eb61..928a705775f 100644 --- a/apps/sim/ee/credential-groups/components/organization-account-people.test.tsx +++ b/apps/sim/ee/credential-groups/components/organization-account-people.test.tsx @@ -126,6 +126,58 @@ it('keeps the compact People rows and resends from the actions menu', async () = ) }) +it('counts active accounts and separates disconnected and reconnecting providers', async () => { + mocks.people.mockReturnValue({ + data: { + pages: [ + { + enrollments: [ + { + id: 'enrollment-1', + email: 'person@example.com', + status: 'completed', + connections: [ + { provider: 'google-calendar', status: 'revoked', count: 3 }, + { provider: 'gmail', status: 'active', count: 2 }, + { provider: 'google-drive', status: 'needs_reauth', count: 1 }, + ], + mcpConnections: [ + { mcpServerId: 'active-server', name: 'Research workspace', status: 'active' }, + { mcpServerId: 'revoked-server', name: 'Archived workspace', status: 'revoked' }, + ], + }, + ], + }, + ], + }, + }) + await renderPeople() + + expect(container.textContent).toContain('3 accounts connected') + const groups = Array.from(container.querySelectorAll('[role="group"]')) + expect(groups.map((group) => group.getAttribute('aria-label'))).toEqual([ + 'Connected', + 'Reconnect required', + 'Disconnected', + ]) + expect(groups[0]?.textContent).toContain('Gmail (2)') + expect(groups[0]?.textContent).toContain('Research workspace') + expect(groups[0]?.textContent).not.toContain('Calendar') + expect(groups[1]?.textContent).toContain('Google Drive') + expect(groups[2]?.textContent).toContain('Google Calendar (3)') + expect(groups[2]?.textContent).toContain('Archived workspace') +}) + +it('hides stale connected badges after the person’s access is revoked', async () => { + const result = mocks.people() + result.data.pages[0].enrollments[0].status = 'revoked' + await renderPeople() + + expect(container.textContent).toContain('Access revoked') + expect(container.textContent).not.toContain('accounts connected') + expect(container.querySelector('[role="group"]')).toBeNull() +}) + it('requires revoke confirmation, allows cancellation, and never submits from an unfocused Enter', async () => { await renderPeople() await selectPersonAction('Revoke') diff --git a/apps/sim/ee/credential-groups/components/organization-account-people.tsx b/apps/sim/ee/credential-groups/components/organization-account-people.tsx index 5648b046e6b..cb62e2f5ee4 100644 --- a/apps/sim/ee/credential-groups/components/organization-account-people.tsx +++ b/apps/sim/ee/credential-groups/components/organization-account-people.tsx @@ -1,11 +1,10 @@ 'use client' import { type ReactNode, useState } from 'react' -import { Chip, ChipConfirmModal, ChipModalError, toast } from '@sim/emcn' +import { Avatar, AvatarFallback, Chip, ChipConfirmModal, ChipModalError, toast } from '@sim/emcn' import { Plus } from '@sim/emcn/icons' import type { SettingsAction, SettingsBackAction } from '@/components/settings/settings-header' import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' -import { MemberAvatar } from '@/app/workspace/[workspaceId]/settings/components/member-list' import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu' import { SettingsEmptyState, @@ -17,7 +16,10 @@ import { SettingsResourceRow, } from '@/app/workspace/[workspaceId]/settings/components/settings-resource-row' import { OrganizationAccountInviteModal } from '@/ee/credential-groups/components/organization-account-invite-modal' -import { OrganizationPersonConnections } from '@/ee/credential-groups/components/organization-person-connections' +import { + getOrganizationPersonConnectionSummary, + OrganizationPersonConnections, +} from '@/ee/credential-groups/components/organization-person-connections' import { useOrganizationAccountPeople, useResendOrganizationAccountInvitation, @@ -110,44 +112,52 @@ export function OrganizationAccountPeople({ ) : (
{enrollments.map((person) => ( - } - iconVariant='custom' - title={person.email} - description={} - trailing={ - - resend.mutate( - { - organizationId, - enrollmentId: person.id, - ...(searchConnection - ? { optionId: searchConnection.optionId } - : {}), - }, - { onSuccess: () => toast.success('Invitation sent') } - ), - }, - { - label: searchConnection ? 'Revoke all account access' : 'Revoke', - destructive: true, - disabled: pending || person.status === 'revoked', - onSelect: () => { - revoke.reset() - setRevokingPerson({ id: person.id, email: person.email }) +
+ + {person.email.charAt(0).toUpperCase()} + + } + iconVariant='custom' + title={person.email} + description={getOrganizationPersonConnectionSummary(person)} + trailing={ + + resend.mutate( + { + organizationId, + enrollmentId: person.id, + ...(searchConnection + ? { optionId: searchConnection.optionId } + : {}), + }, + { onSuccess: () => toast.success('Invitation sent') } + ), + }, + { + label: searchConnection ? 'Revoke all account access' : 'Revoke', + destructive: true, + disabled: pending || person.status === 'revoked', + onSelect: () => { + revoke.reset() + setRevokingPerson({ id: person.id, email: person.email }) + }, }, - }, - ]} - /> - } - /> + ]} + /> + } + /> +
+ +
+
))}
)} diff --git a/apps/sim/ee/credential-groups/components/organization-person-connections.tsx b/apps/sim/ee/credential-groups/components/organization-person-connections.tsx index 1654bbf830a..b01b9979f04 100644 --- a/apps/sim/ee/credential-groups/components/organization-person-connections.tsx +++ b/apps/sim/ee/credential-groups/components/organization-person-connections.tsx @@ -1,4 +1,4 @@ -import { ChipTag } from '@sim/emcn' +import { ChipTag, OverflowText } from '@sim/emcn' import type { CredentialGroupEnrollmentDetail } from '@/lib/api/contracts/credential-groups' import { getCredentialGroupProviderService } from '@/lib/credential-groups/providers' import { resolveCredentialDisplay } from '@/lib/integrations/credential-display' @@ -13,8 +13,26 @@ const CONNECTION_STATUS_LABELS = { revoked: 'Disconnected', } as const +export function getOrganizationPersonConnectionSummary(person: CredentialGroupEnrollmentDetail) { + if (person.status === 'revoked') return 'Access revoked' + + const connected = + person.connections.reduce( + (count, connection) => count + (connection.status === 'active' ? connection.count : 0), + 0 + ) + person.mcpConnections.filter((connection) => connection.status === 'active').length + if (connected > 0) return `${connected} ${connected === 1 ? 'account' : 'accounts'} connected` + + const statuses = [...person.connections, ...person.mcpConnections].map(({ status }) => status) + if (statuses.includes('needs_reauth')) return 'Reconnect required' + if (statuses.includes('revoked')) return 'Disconnected' + if (person.status === 'delivery_failed') return 'Connection request failed' + if (person.expired) return 'Connection request expired' + return 'Not connected' +} + export function OrganizationPersonConnections({ person }: OrganizationPersonConnectionsProps) { - if (person.status === 'revoked') return <>Access revoked + if (person.status === 'revoked') return null const connections = person.connections.map((connection) => { const display = @@ -32,55 +50,44 @@ export function OrganizationPersonConnections({ person }: OrganizationPersonConn return { key: `${connection.provider}:${connection.status}`, name: service?.name ?? display?.detailTitle ?? 'GitLab', - icon: service?.icon ?? display?.icon, + icon: service?.icon ?? display?.icon ?? undefined, status: connection.status, count: connection.count, } }) - const total = - connections.reduce( - (count, connection) => count + (connection.status === 'active' ? connection.count : 0), - 0 - ) + person.mcpConnections.filter((connection) => connection.status === 'active').length - - const statuses = [...connections, ...person.mcpConnections].map(({ status }) => status) + const accounts = [ + ...connections, + ...person.mcpConnections.map((connection) => ({ + key: `mcp:${connection.mcpServerId}`, + name: connection.name, + status: connection.status, + count: 1, + icon: undefined, + })), + ] return ( - - - {total > 0 - ? `${total} ${total === 1 ? 'account' : 'accounts'} connected` - : statuses.includes('needs_reauth') - ? 'Reconnect required' - : statuses.includes('revoked') - ? 'Disconnected' - : person.status === 'delivery_failed' - ? 'Connection request failed' - : person.expired - ? 'Connection request expired' - : 'Not connected'} - - {(connections.length > 0 || person.mcpConnections.length > 0) && ( - - {connections.map(({ key, name, icon: Icon, status, count }) => ( - - {Icon && } - {name} - {count > 1 ? ` (${count})` : ''} - {status !== 'active' && ` · ${CONNECTION_STATUS_LABELS[status]}`} - - ))} - {person.mcpConnections.map((connection) => ( - - {connection.name} · {CONNECTION_STATUS_LABELS[connection.status]} - - ))} - - )} - +
+ {(['active', 'needs_reauth', 'revoked'] as const).map((status) => { + const matching = accounts.filter((account) => account.status === status) + if (matching.length === 0) return null + return ( +
+ {status !== 'active' && ( +

+ {CONNECTION_STATUS_LABELS[status]} +

+ )} +
+ {matching.map(({ key, name, icon, count }) => ( + + 1 ? `${name} (${count})` : name} /> + + ))} +
+
+ ) + })} +
) } diff --git a/apps/sim/executor/handlers/credential/credential-handler.test.ts b/apps/sim/executor/handlers/credential/credential-handler.test.ts index 417616009af..3b9bcf0951e 100644 --- a/apps/sim/executor/handlers/credential/credential-handler.test.ts +++ b/apps/sim/executor/handlers/credential/credential-handler.test.ts @@ -42,7 +42,7 @@ describe('Credential organization operations', () => { vi.clearAllMocks() mocks.principal.mockResolvedValue({ delegationId: 'current-run' }) mocks.oauth.mockResolvedValue({ - credentials: [account], + credentials: [{ ...account, accountEmail: 'personal@example.com' }], count: 1, hasMore: false, nextCursor: null, @@ -103,6 +103,85 @@ describe('Credential organization operations', () => { }) ) }) + it('discovers provider emails across pages without requiring an enrollment email', async () => { + const accounts = [ + { ...account, accountEmail: 'first@example.com' }, + { ...account, credentialId: 'credential-2', accountEmail: 'second@example.com' }, + { + ...account, + credentialId: 'credential-3', + email: 'colleague@example.com', + accountEmail: 'third@example.com', + }, + ] + mocks.oauth + .mockResolvedValueOnce({ + credentials: accounts.slice(0, 2), + count: 2, + hasMore: true, + nextCursor: 'credential-2', + }) + .mockResolvedValueOnce({ + credentials: accounts.slice(2), + count: 1, + hasMore: false, + nextCursor: null, + }) + const input = { + operation: 'list_organization_accounts', + organizationProviders: ['google-email'], + limit: 2, + } + const first = await handler.execute(ctx, block, input) + const second = await handler.execute(ctx, block, { ...input, cursor: first.nextCursor }) + expect(first).toMatchObject({ + credentials: accounts.slice(0, 2), + emails: ['first@example.com', 'second@example.com'], + count: 2, + hasMore: true, + }) + expect(second).toMatchObject({ + credentials: accounts.slice(2), + emails: ['third@example.com'], + hasMore: false, + nextCursor: null, + }) + expect(mocks.oauth).toHaveBeenLastCalledWith({ + principal: { delegationId: 'current-run' }, + input: { + workspaceId: 'child-workspace', + email: undefined, + credentialProviderIds: ['google-email'], + limit: 2, + cursor: 'credential-2', + }, + }) + }) + it('preserves the optional exact enrollment-email filter for organization lists', async () => { + await handler.execute(ctx, block, { + operation: 'list_organization_accounts', + organizationProviders: ['google-email'], + email: 'person@example.com', + }) + expect(mocks.oauth).toHaveBeenCalledWith( + expect.objectContaining({ input: expect.objectContaining({ email: 'person@example.com' }) }) + ) + }) + it('returns empty email and account arrays when no accessible accounts match', async () => { + mocks.oauth.mockResolvedValue({ credentials: [], count: 0, hasMore: false, nextCursor: null }) + await expect( + handler.execute(ctx, block, { + operation: 'list_organization_accounts', + organizationProviders: ['google-email'], + }) + ).resolves.toEqual({ + credentials: [], + emails: [], + count: 0, + hasMore: false, + nextCursor: null, + }) + }) it('returns the person’s MCP credential separately from the shared server', async () => { const connection = { credentialId: 'mcp-cg-person', diff --git a/apps/sim/executor/handlers/credential/credential-handler.ts b/apps/sim/executor/handlers/credential/credential-handler.ts index f570e00aa73..ac7b0f888a9 100644 --- a/apps/sim/executor/handlers/credential/credential-handler.ts +++ b/apps/sim/executor/handlers/credential/credential-handler.ts @@ -1,3 +1,4 @@ +import { omit } from '@sim/utils/object' import { CREDENTIAL_GROUP_DELEGATION_AUDIENCE } from '@/lib/credential-groups/application/authorization' import { listCredentialGroupCredentials } from '@/lib/credential-groups/application/list-credentials' import { listCredentialGroupMcpConnections } from '@/lib/credential-groups/application/list-mcp-connections' @@ -114,12 +115,14 @@ export class CredentialBlockHandler implements BlockHandler { cursor: find ? undefined : parseOptionalString(inputs.cursor, 'Cursor'), }, }) - if (!find) return result + if (!find) { + return { ...result, emails: result.credentials.map((account) => account.accountEmail) } + } if (result.credentials.length !== 1 || result.hasMore) throw new Error( `Expected exactly one organization account; found ${result.credentials.length}${result.hasMore ? '+' : ''}. Check the email, provider, and connection status.` ) - return result.credentials[0]! + return omit(result.credentials[0]!, ['accountEmail']) } case 'find_organization_mcp_connection': case 'list_organization_mcp_connections': { diff --git a/apps/sim/lib/credential-groups/application/list-credentials.test.ts b/apps/sim/lib/credential-groups/application/list-credentials.test.ts index f0c73b24891..ae3ed7fcca4 100644 --- a/apps/sim/lib/credential-groups/application/list-credentials.test.ts +++ b/apps/sim/lib/credential-groups/application/list-credentials.test.ts @@ -136,6 +136,7 @@ describe('listCredentialGroupCredentials', () => { { credentialId: 'credential-1', email: 'person@example.com', + accountEmail: 'personal@example.com', displayName: 'person@example.com', providerId: 'google-email', providerSubjectId: 'google-subject-1', @@ -254,6 +255,7 @@ describe('listCredentialGroupCredentials', () => { { credentialId: 'credential-1', email: 'person@example.com', + accountEmail: 'personal@example.com', displayName: 'person@example.com', providerId: 'google-email', providerSubjectId: 'google-subject-1', @@ -307,6 +309,34 @@ describe('listCredentialGroupCredentials', () => { ) }) + it('rechecks a provider grant before the next page can expose account identities', async () => { + mocks.loadGroup.mockResolvedValue({ + ...groupContext, + options: [ + ...groupContext.options, + { ...groupContext.options[0], id: 'calendar-option', provider: 'google-calendar' }, + ], + }) + const query = { ...input, credentialProviderIds: ['google-email'] } + await listCredentialGroupCredentials.execute({ principal: executorPrincipal(), input: query }) + mocks.listCredentials.mockClear() + mocks.requirePolicy.mockResolvedValue({ + document: buildOrganizationAccountAccessPolicy('group-1', [ + { + workspaceId: 'workspace-1', + access: { mode: 'selected', credentialTypes: ['oauth:google-calendar'] }, + }, + ]), + }) + await expect( + listCredentialGroupCredentials.execute({ + principal: executorPrincipal(), + input: { ...query, cursor: 'credential-1' }, + }) + ).rejects.toMatchObject({ code: 'forbidden' }) + expect(mocks.listCredentials).not.toHaveBeenCalled() + }) + it('normalizes an optional email filter independently of caller identity', async () => { await listCredentialGroupCredentials.execute({ principal: executorPrincipal(), diff --git a/apps/sim/lib/credential-groups/application/list-credentials.ts b/apps/sim/lib/credential-groups/application/list-credentials.ts index 203b43c63af..68fbde5cb1b 100644 --- a/apps/sim/lib/credential-groups/application/list-credentials.ts +++ b/apps/sim/lib/credential-groups/application/list-credentials.ts @@ -13,9 +13,9 @@ import { import { organizationAccountPolicyAllowsWorkspace } from '@/lib/credential-groups/application/workspace-access-policy' import { CredentialGroupCredentialCursorNotFoundError, - type CredentialGroupCredentialReference, listCredentialGroupCredentialReferences, MAX_CREDENTIAL_GROUP_CREDENTIAL_PAGE_SIZE, + type OrganizationAccountCredentialReference, } from '@/lib/credential-groups/credentials' import { getCredentialGroupProviderId, @@ -31,7 +31,7 @@ export interface ListCredentialGroupCredentialsInput { } export interface ListCredentialGroupCredentialsResult { - credentials: CredentialGroupCredentialReference[] + credentials: OrganizationAccountCredentialReference[] count: number hasMore: boolean nextCursor: string | null diff --git a/apps/sim/lib/credential-groups/credentials.test.ts b/apps/sim/lib/credential-groups/credentials.test.ts index 98077cea353..279bedd3326 100644 --- a/apps/sim/lib/credential-groups/credentials.test.ts +++ b/apps/sim/lib/credential-groups/credentials.test.ts @@ -1,6 +1,8 @@ /** * @vitest-environment node */ + +import { credential, credentialGroupEnrollment } from '@sim/db/schema' import { dbChainMockFns, hasMockCondition, resetDbChainMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -10,6 +12,7 @@ vi.mock('@/lib/credential-groups/providers', () => ({ })) import { + CredentialGroupCredentialCursorNotFoundError, listCredentialGroupCredentialReferences, loadCredentialGroupEnrollmentAccessForSubject, } from '@/lib/credential-groups/credentials' @@ -20,11 +23,12 @@ describe('listCredentialGroupCredentialReferences', () => { resetDbChainMock() }) - it('returns the invited email associated with each managed credential', async () => { + it('keeps the enrollment email separate from the verified provider account email', async () => { dbChainMockFns.limit.mockResolvedValueOnce([ { id: 'credential-1', email: 'person@example.com', + accountEmail: 'personal@example.com', displayName: 'Personal Gmail', providerId: 'google-email', providerSubjectId: 'google-subject-1', @@ -45,6 +49,7 @@ describe('listCredentialGroupCredentialReferences', () => { { credentialId: 'credential-1', email: 'person@example.com', + accountEmail: 'personal@example.com', displayName: 'Personal Gmail', providerId: 'google-email', providerSubjectId: 'google-subject-1', @@ -55,6 +60,119 @@ describe('listCredentialGroupCredentialReferences', () => { }) }) + it('lists every provider account across pages without an email filter or secret projection', async () => { + const row = (id: string, accountEmail: string) => ({ + id, + email: 'person@example.com', + accountEmail, + displayName: accountEmail, + providerId: 'google-email', + providerSubjectId: `subject-${id}`, + providerTenantId: null, + createdAt: new Date('2026-08-12T12:00:00.000Z'), + }) + const rows = [ + row('first', 'one@example.com'), + row('second', 'two@example.com'), + row('third', 'three@example.com'), + ] + dbChainMockFns.limit.mockResolvedValueOnce(rows) + const input = { + organizationId: 'organization-1', + credentialGroupId: 'group-1', + credentialGroupOptionIds: ['gmail-option'], + credentialProviderIds: ['google-email'], + limit: 2, + } + const first = await listCredentialGroupCredentialReferences(input) + expect(first.nextCursor).toBe('second') + dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'second' }]).mockResolvedValueOnce([rows[2]]) + const second = await listCredentialGroupCredentialReferences({ + ...input, + cursor: first.nextCursor!, + }) + expect(second.nextCursor).toBeNull() + expect( + [...first.credentials, ...second.credentials].map((account) => account.accountEmail) + ).toEqual(['one@example.com', 'two@example.com', 'three@example.com']) + expect(dbChainMockFns.limit.mock.calls.map(([limit]) => limit)).toEqual([3, 1, 3]) + + for (const [where] of dbChainMockFns.where.mock.calls) { + for (const [column, value] of [ + [credential.organizationId, 'organization-1'], + [credentialGroupEnrollment.credentialGroupId, 'group-1'], + [credential.managedOauthStatus, 'active'], + [credential.createdBy, credentialGroupEnrollment.userId], + ]) { + expect( + hasMockCondition( + where, + (condition) => + condition.type === 'eq' && condition.left === column && condition.right === value + ) + ).toBe(true) + } + expect( + hasMockCondition( + where, + (condition) => + condition.type === 'inArray' && + condition.column === credential.credentialGroupOptionId && + JSON.stringify(condition.values) === '["gmail-option"]' + ) + ).toBe(true) + expect( + hasMockCondition( + where, + (condition) => + condition.type === 'eq' && condition.left === credentialGroupEnrollment.email + ) + ).toBe(false) + } + expect(Object.keys(dbChainMockFns.select.mock.calls[0]![0])).toEqual([ + 'id', + 'email', + 'accountEmail', + 'displayName', + 'providerId', + 'providerSubjectId', + 'providerTenantId', + 'managedOauthStatus', + 'enrollmentStatus', + 'createdAt', + ]) + }) + + it.each([null, '', 'not-an-email'])( + 'fails fast when the provider account email is invalid: %s', + async (accountEmail) => { + dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'credential-1', accountEmail }]) + await expect( + listCredentialGroupCredentialReferences({ + organizationId: 'organization-1', + credentialGroupId: 'group-1', + credentialGroupOptionIds: ['gmail-option'], + limit: 50, + }) + ).rejects.toThrow('no valid provider account email') + } + ) + + it('rejects a cursor outside the current provider and organization scope before reading a page', async () => { + dbChainMockFns.limit.mockResolvedValueOnce([]) + await expect( + listCredentialGroupCredentialReferences({ + organizationId: 'organization-1', + credentialGroupId: 'group-1', + credentialGroupOptionIds: ['gmail-option'], + credentialProviderIds: ['google-email'], + cursor: 'foreign-credential', + limit: 2, + }) + ).rejects.toBeInstanceOf(CredentialGroupCredentialCursorNotFoundError) + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + it('filters credential references by normalized enrollment email', async () => { dbChainMockFns.limit.mockResolvedValueOnce([]) diff --git a/apps/sim/lib/credential-groups/credentials.ts b/apps/sim/lib/credential-groups/credentials.ts index ec0fe7889af..fb7d0d4af20 100644 --- a/apps/sim/lib/credential-groups/credentials.ts +++ b/apps/sim/lib/credential-groups/credentials.ts @@ -7,6 +7,7 @@ import { credentialGroupEnrollment, user, } from '@sim/db/schema' +import { isValidEmailSyntax } from '@sim/utils/string' import { and, asc, eq, gt, inArray, isNotNull, isNull, or, type SQL, sql } from 'drizzle-orm' import { type ResourceScope, resourceScopeFromOwner } from '@/lib/core/resource-scope' import { resourceScopeCondition } from '@/lib/core/resource-scope.server' @@ -37,6 +38,10 @@ export interface CredentialGroupCredentialReference { providerTenantId: string | null } +export interface OrganizationAccountCredentialReference extends CredentialGroupCredentialReference { + accountEmail: string +} + /** * A credential collected under one option, in any state. Carries both statuses * so a caller reconciling membership can tell a live credential from one that @@ -285,6 +290,7 @@ export async function loadManagedCredentialGroupBinding( interface CredentialReferencePageRow { id: string email: string + accountEmail: string | null displayName: string providerId: string | null providerSubjectId: string | null @@ -325,6 +331,7 @@ async function pageCredentialReferences( .select({ id: credential.id, email: credentialGroupEnrollment.email, + accountEmail: sql`${credential.providerMetadata}->>'email'`, displayName: credential.displayName, providerId: credential.providerId, providerSubjectId: credential.providerSubjectId, @@ -396,7 +403,7 @@ export async function listCredentialGroupCredentialReferences({ credentialProviderIds, credentialGroupOptionIds, }: ListCredentialGroupCredentialReferencesInput): Promise<{ - credentials: CredentialGroupCredentialReference[] + credentials: OrganizationAccountCredentialReference[] nextCursor: string | null }> { if (credentialGroupOptionIds.length === 0) { @@ -426,7 +433,15 @@ export async function listCredentialGroupCredentialReferences({ limit, cursor ) - return { credentials: page.rows.map(toCredentialReference), nextCursor: page.nextCursor } + return { + credentials: page.rows.map((row) => { + if (!row.accountEmail || !isValidEmailSyntax(row.accountEmail)) { + throw new Error(`Managed credential ${row.id} has no valid provider account email`) + } + return { ...toCredentialReference(row), accountEmail: row.accountEmail } + }), + nextCursor: page.nextCursor, + } } /** diff --git a/apps/sim/scripts/block-registry-source.test.ts b/apps/sim/scripts/block-registry-source.test.ts new file mode 100644 index 00000000000..d8fc55680b2 --- /dev/null +++ b/apps/sim/scripts/block-registry-source.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest' +import { extractInlineBlockSubBlockIds } from '@/scripts/block-registry-source' + +describe('extractInlineBlockSubBlockIds', () => { + it('does not attribute a satisfies-style legacy block’s fields to a typed successor', () => { + expect( + extractInlineBlockSubBlockIds(` + export const LegacyBlock = { + type: 'legacy', + subBlocks: [{ id: 'operation' }, { id: 'encoding' }], + } satisfies BlockConfig + export const CurrentBlock: BlockConfig = { + ...LegacyBlock, + type: 'current', + subBlocks: LegacyBlock.subBlocks.filter((field) => field.id !== 'encoding'), + } + `) + ).toEqual({ legacy: ['operation', 'encoding'] }) + }) + + it('reads each inline definition and ignores nested option IDs and punctuation in strings', () => { + expect( + extractInlineBlockSubBlockIds(` + export const FirstBlock: BlockConfig = { + type: 'first', subBlocks: [{ id: 'operation', options: [{ id: 'nested' }] }], + } + export const SecondBlock: BlockConfig = { + type: 'second', subBlocks: [ + { id: 'template', placeholder: 'objects: [{ }]' }, + { id: 'text' }, + ], + } + `) + ).toEqual({ first: ['operation'], second: ['template', 'text'] }) + }) +}) diff --git a/apps/sim/scripts/block-registry-source.ts b/apps/sim/scripts/block-registry-source.ts new file mode 100644 index 00000000000..2fb155df7e6 --- /dev/null +++ b/apps/sim/scripts/block-registry-source.ts @@ -0,0 +1,51 @@ +import ts from '@typescript/typescript6' + +function unwrap(expression: ts.Expression): ts.Expression { + while ( + ts.isSatisfiesExpression(expression) || + ts.isAsExpression(expression) || + ts.isParenthesizedExpression(expression) + ) { + expression = expression.expression + } + return expression +} + +function property(object: ts.ObjectLiteralExpression, name: string): ts.Expression | undefined { + const member = object.properties.find( + (member) => + ts.isPropertyAssignment(member) && + (ts.isIdentifier(member.name) || ts.isStringLiteral(member.name)) && + member.name.text === name + ) + return member && ts.isPropertyAssignment(member) ? member.initializer : undefined +} + +/** Extracts inline subblock IDs from their owning declaration, including satisfies-style blocks. */ +export function extractInlineBlockSubBlockIds(source: string): Record { + const file = ts.createSourceFile('block.ts', source, ts.ScriptTarget.Latest, true) + const blocks: Record = {} + for (const statement of file.statements) { + if (!ts.isVariableStatement(statement)) continue + for (const declaration of statement.declarationList.declarations) { + if (!declaration.initializer) continue + const object = unwrap(declaration.initializer) + if (!ts.isObjectLiteralExpression(object)) continue + const type = property(object, 'type') + const subBlocks = property(object, 'subBlocks') + if ( + !type || + !ts.isStringLiteral(type) || + !subBlocks || + !ts.isArrayLiteralExpression(subBlocks) + ) + continue + blocks[type.text] = subBlocks.elements.flatMap((element) => { + if (!ts.isObjectLiteralExpression(element)) return [] + const id = property(element, 'id') + return id && ts.isStringLiteral(id) ? [id.text] : [] + }) + } + } + return blocks +} diff --git a/apps/sim/scripts/check-block-registry.ts b/apps/sim/scripts/check-block-registry.ts index 6264d8ebaad..de69213768a 100644 --- a/apps/sim/scripts/check-block-registry.ts +++ b/apps/sim/scripts/check-block-registry.ts @@ -27,6 +27,7 @@ import { execSync } from 'child_process' import { SUBBLOCK_ID_MIGRATIONS } from '@/lib/workflows/migrations/subblock-migrations' import { getAllBlocks, getBlock, getBlockMeta } from '@/blocks/registry' +import { extractInlineBlockSubBlockIds } from '@/scripts/block-registry-source' import { getToolParams } from '@/tools/metadata' const baseRef = process.argv[2] || 'HEAD~1' @@ -36,50 +37,6 @@ const gitOpts = { encoding: 'utf-8' as const, cwd: gitRoot } type IdMap = Record> -/** - * Extracts subblock IDs from the `subBlocks: [ ... ]` section of a block - * definition. Only grabs the top-level `id:` of each subblock object — - * ignores nested IDs inside `options`, `columns`, etc. - */ -function extractSubBlockIds(source: string): string[] { - const startIdx = source.indexOf('subBlocks:') - if (startIdx === -1) return [] - - const bracketStart = source.indexOf('[', startIdx) - if (bracketStart === -1) return [] - - const ids: string[] = [] - let braceDepth = 0 - let bracketDepth = 0 - let i = bracketStart + 1 - bracketDepth = 1 - - while (i < source.length && bracketDepth > 0) { - const ch = source[i] - - if (ch === '[') bracketDepth++ - else if (ch === ']') { - bracketDepth-- - if (bracketDepth === 0) break - } else if (ch === '{') { - braceDepth++ - if (braceDepth === 1) { - const ahead = source.slice(i, i + 200) - const idMatch = ahead.match(/{\s*(?:\/\/[^\n]*\n\s*)*id:\s*['"]([^'"]+)['"]/) - if (idMatch) { - ids.push(idMatch[1]) - } - } - } else if (ch === '}') { - braceDepth-- - } - - i++ - } - - return ids -} - function getCurrentIds(): IdMap { const map: IdMap = {} for (const block of getAllBlocks()) { @@ -128,16 +85,9 @@ function getPreviousIds(): PreviousIdsResult { continue } - const typeMatch = content.match( - /BlockConfig(?:<[^>]*>)?\s*=\s*\{[\s\S]*?type:\s*['"]([^'"]+)['"]/ - ) - if (!typeMatch) continue - const blockType = typeMatch[1] - - const ids = extractSubBlockIds(content) - if (ids.length === 0) continue - - map[blockType] = new Set(ids) + for (const [blockType, ids] of Object.entries(extractInlineBlockSubBlockIds(content))) { + if (ids.length > 0) map[blockType] = new Set(ids) + } } } catch (err) { return { kind: 'skip', reason: `Could not read previous block files from ${baseRef}: ${err}` } From 2081acc63f8027a66bcd5325fee422e04bef64f7 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Wed, 16 Sep 2026 18:16:09 -0700 Subject: [PATCH 2/3] fix(credential-groups): keep people rows compact and active only --- .../organization-account-people.test.tsx | 41 +++--- .../organization-account-people.tsx | 105 ++++++------- .../organization-person-connections.tsx | 139 ++++++++---------- 3 files changed, 137 insertions(+), 148 deletions(-) diff --git a/apps/sim/ee/credential-groups/components/organization-account-people.test.tsx b/apps/sim/ee/credential-groups/components/organization-account-people.test.tsx index 928a705775f..40748c30928 100644 --- a/apps/sim/ee/credential-groups/components/organization-account-people.test.tsx +++ b/apps/sim/ee/credential-groups/components/organization-account-people.test.tsx @@ -115,7 +115,7 @@ it('keeps the compact People rows and resends from the actions menu', async () = ) ) - expect(container.textContent).toContain('2 accounts connected') + expect(container.textContent).toContain('Gmail (2)') expect(container.textContent).not.toContain('Copy new link') expect(container.textContent).not.toContain('gmail: active') expect(container.textContent).not.toContain('People (1)') @@ -126,7 +126,7 @@ it('keeps the compact People rows and resends from the actions menu', async () = ) }) -it('counts active accounts and separates disconnected and reconnecting providers', async () => { +it('shows only active OAuth and MCP accounts', async () => { mocks.people.mockReturnValue({ data: { pages: [ @@ -153,19 +153,14 @@ it('counts active accounts and separates disconnected and reconnecting providers }) await renderPeople() - expect(container.textContent).toContain('3 accounts connected') - const groups = Array.from(container.querySelectorAll('[role="group"]')) - expect(groups.map((group) => group.getAttribute('aria-label'))).toEqual([ - 'Connected', - 'Reconnect required', - 'Disconnected', - ]) - expect(groups[0]?.textContent).toContain('Gmail (2)') - expect(groups[0]?.textContent).toContain('Research workspace') - expect(groups[0]?.textContent).not.toContain('Calendar') - expect(groups[1]?.textContent).toContain('Google Drive') - expect(groups[2]?.textContent).toContain('Google Calendar (3)') - expect(groups[2]?.textContent).toContain('Archived workspace') + const group = container.querySelector('[aria-label="Connected accounts"]') + expect(group?.textContent).toContain('Gmail (2)') + expect(group?.textContent).toContain('Research workspace') + expect(container.textContent).not.toContain('Google Calendar') + expect(container.textContent).not.toContain('Google Drive') + expect(container.textContent).not.toContain('Archived workspace') + expect(container.textContent).not.toContain('Disconnected') + expect(container.textContent).not.toContain('Reconnect required') }) it('hides stale connected badges after the person’s access is revoked', async () => { @@ -173,7 +168,8 @@ it('hides stale connected badges after the person’s access is revoked', async result.data.pages[0].enrollments[0].status = 'revoked' await renderPeople() - expect(container.textContent).toContain('Access revoked') + expect(container.textContent).toContain('person@example.com') + expect(container.textContent).not.toContain('Gmail') expect(container.textContent).not.toContain('accounts connected') expect(container.querySelector('[role="group"]')).toBeNull() }) @@ -450,12 +446,12 @@ it('keeps a failed revoke confirmation open for retry and blocks dismissal while }) it.each([ - ['invited', [], 'Not connected'], - ['completed', [{ provider: 'gmail', status: 'needs_reauth', count: 1 }], 'Reconnect required'], - ['revoked', [], 'Access revoked'], + ['invited', []], + ['completed', [{ provider: 'gmail', status: 'needs_reauth', count: 1 }]], + ['revoked', []], ])( - 'preserves provider navigation and exposes an honest connection state: %s', - async (status, connections, label) => { + 'preserves provider navigation and hides inactive account badges: %s', + async (status, connections) => { mocks.people.mockReturnValue({ data: { pages: [ @@ -494,7 +490,8 @@ it.each([ optionId: 'gmail-option', }) expect(container.textContent).toContain('Gmail') - expect(container.textContent).toContain(label) + expect(container.textContent).toContain('person@example.com') + expect(container.querySelector('[aria-label="Connected accounts"]')).toBeNull() expect(container.textContent).not.toContain('No people invited') } ) diff --git a/apps/sim/ee/credential-groups/components/organization-account-people.tsx b/apps/sim/ee/credential-groups/components/organization-account-people.tsx index cb62e2f5ee4..ad2481f5974 100644 --- a/apps/sim/ee/credential-groups/components/organization-account-people.tsx +++ b/apps/sim/ee/credential-groups/components/organization-account-people.tsx @@ -1,7 +1,15 @@ 'use client' import { type ReactNode, useState } from 'react' -import { Avatar, AvatarFallback, Chip, ChipConfirmModal, ChipModalError, toast } from '@sim/emcn' +import { + Avatar, + AvatarFallback, + Chip, + ChipConfirmModal, + ChipModalError, + OverflowText, + toast, +} from '@sim/emcn' import { Plus } from '@sim/emcn/icons' import type { SettingsAction, SettingsBackAction } from '@/components/settings/settings-header' import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' @@ -16,10 +24,7 @@ import { SettingsResourceRow, } from '@/app/workspace/[workspaceId]/settings/components/settings-resource-row' import { OrganizationAccountInviteModal } from '@/ee/credential-groups/components/organization-account-invite-modal' -import { - getOrganizationPersonConnectionSummary, - OrganizationPersonConnections, -} from '@/ee/credential-groups/components/organization-person-connections' +import { OrganizationPersonConnections } from '@/ee/credential-groups/components/organization-person-connections' import { useOrganizationAccountPeople, useResendOrganizationAccountInvitation, @@ -112,52 +117,52 @@ export function OrganizationAccountPeople({ ) : (
{enrollments.map((person) => ( -
- - {person.email.charAt(0).toUpperCase()} - - } - iconVariant='custom' - title={person.email} - description={getOrganizationPersonConnectionSummary(person)} - trailing={ - - resend.mutate( - { - organizationId, - enrollmentId: person.id, - ...(searchConnection - ? { optionId: searchConnection.optionId } - : {}), - }, - { onSuccess: () => toast.success('Invitation sent') } - ), - }, - { - label: searchConnection ? 'Revoke all account access' : 'Revoke', - destructive: true, - disabled: pending || person.status === 'revoked', - onSelect: () => { - revoke.reset() - setRevokingPerson({ id: person.id, email: person.email }) - }, + + {person.email.charAt(0).toUpperCase()} + + } + iconVariant='custom' + title={ + + + + + } + trailing={ + + resend.mutate( + { + organizationId, + enrollmentId: person.id, + ...(searchConnection + ? { optionId: searchConnection.optionId } + : {}), + }, + { onSuccess: () => toast.success('Invitation sent') } + ), + }, + { + label: searchConnection ? 'Revoke all account access' : 'Revoke', + destructive: true, + disabled: pending || person.status === 'revoked', + onSelect: () => { + revoke.reset() + setRevokingPerson({ id: person.id, email: person.email }) }, - ]} - /> - } - /> -
- -
-
+ }, + ]} + /> + } + /> ))}
)} diff --git a/apps/sim/ee/credential-groups/components/organization-person-connections.tsx b/apps/sim/ee/credential-groups/components/organization-person-connections.tsx index b01b9979f04..ede12506eaf 100644 --- a/apps/sim/ee/credential-groups/components/organization-person-connections.tsx +++ b/apps/sim/ee/credential-groups/components/organization-person-connections.tsx @@ -1,4 +1,14 @@ -import { ChipTag, OverflowText } from '@sim/emcn' +'use client' + +import { useRef } from 'react' +import { + ChipTag, + cn, + OverflowText, + scrollFadeAttributes, + scrollFadeXClass, + useScrollEdges, +} from '@sim/emcn' import type { CredentialGroupEnrollmentDetail } from '@/lib/api/contracts/credential-groups' import { getCredentialGroupProviderService } from '@/lib/credential-groups/providers' import { resolveCredentialDisplay } from '@/lib/integrations/credential-display' @@ -7,87 +17,64 @@ interface OrganizationPersonConnectionsProps { person: CredentialGroupEnrollmentDetail } -const CONNECTION_STATUS_LABELS = { - active: 'Connected', - needs_reauth: 'Reconnect required', - revoked: 'Disconnected', -} as const - -export function getOrganizationPersonConnectionSummary(person: CredentialGroupEnrollmentDetail) { - if (person.status === 'revoked') return 'Access revoked' - - const connected = - person.connections.reduce( - (count, connection) => count + (connection.status === 'active' ? connection.count : 0), - 0 - ) + person.mcpConnections.filter((connection) => connection.status === 'active').length - if (connected > 0) return `${connected} ${connected === 1 ? 'account' : 'accounts'} connected` - - const statuses = [...person.connections, ...person.mcpConnections].map(({ status }) => status) - if (statuses.includes('needs_reauth')) return 'Reconnect required' - if (statuses.includes('revoked')) return 'Disconnected' - if (person.status === 'delivery_failed') return 'Connection request failed' - if (person.expired) return 'Connection request expired' - return 'Not connected' -} - export function OrganizationPersonConnections({ person }: OrganizationPersonConnectionsProps) { - if (person.status === 'revoked') return null + const connectionsRef = useRef(null) - const connections = person.connections.map((connection) => { - const display = - connection.provider === 'gitlab' - ? resolveCredentialDisplay({ - type: 'personal_token', - providerId: 'gitlab', - displayName: 'GitLab', - }) - : undefined - const service = - connection.provider !== 'gitlab' - ? getCredentialGroupProviderService(connection.provider) - : undefined - return { - key: `${connection.provider}:${connection.status}`, - name: service?.name ?? display?.detailTitle ?? 'GitLab', - icon: service?.icon ?? display?.icon ?? undefined, - status: connection.status, - count: connection.count, - } - }) + const connections = person.connections + .filter((connection) => connection.status === 'active') + .map((connection) => { + const display = + connection.provider === 'gitlab' + ? resolveCredentialDisplay({ + type: 'personal_token', + providerId: 'gitlab', + displayName: 'GitLab', + }) + : undefined + const service = + connection.provider !== 'gitlab' + ? getCredentialGroupProviderService(connection.provider) + : undefined + return { + key: `${connection.provider}:${connection.status}`, + name: service?.name ?? display?.detailTitle ?? 'GitLab', + icon: service?.icon ?? display?.icon ?? undefined, + count: connection.count, + } + }) const accounts = [ ...connections, - ...person.mcpConnections.map((connection) => ({ - key: `mcp:${connection.mcpServerId}`, - name: connection.name, - status: connection.status, - count: 1, - icon: undefined, - })), + ...person.mcpConnections + .filter((connection) => connection.status === 'active') + .map((connection) => ({ + key: `mcp:${connection.mcpServerId}`, + name: connection.name, + count: 1, + icon: undefined, + })), ] + const visible = person.status !== 'revoked' && accounts.length > 0 + const edges = useScrollEdges(connectionsRef, { axis: 'x', enabled: visible }) + + if (!visible) return null + return ( -
- {(['active', 'needs_reauth', 'revoked'] as const).map((status) => { - const matching = accounts.filter((account) => account.status === status) - if (matching.length === 0) return null - return ( -
- {status !== 'active' && ( -

- {CONNECTION_STATUS_LABELS[status]} -

- )} -
- {matching.map(({ key, name, icon, count }) => ( - - 1 ? `${name} (${count})` : name} /> - - ))} -
-
- ) - })} -
+ + {accounts.map(({ key, name, icon, count }) => ( + + 1 ? `${name} (${count})` : name} /> + + ))} + ) } From 22d9d7a4e13ba783f9b097357ca753657e64c095 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Wed, 16 Sep 2026 18:33:21 -0700 Subject: [PATCH 3/3] fix(audit): compare effective block fields from base snapshots --- .../scripts/block-registry-snapshot.test.ts | 133 +++++++++++++++++ apps/sim/scripts/block-registry-snapshot.ts | 134 ++++++++++++++++++ .../sim/scripts/block-registry-source.test.ts | 36 ----- apps/sim/scripts/block-registry-source.ts | 51 ------- apps/sim/scripts/check-block-registry.ts | 74 +++------- 5 files changed, 283 insertions(+), 145 deletions(-) create mode 100644 apps/sim/scripts/block-registry-snapshot.test.ts create mode 100644 apps/sim/scripts/block-registry-snapshot.ts delete mode 100644 apps/sim/scripts/block-registry-source.test.ts delete mode 100644 apps/sim/scripts/block-registry-source.ts diff --git a/apps/sim/scripts/block-registry-snapshot.test.ts b/apps/sim/scripts/block-registry-snapshot.test.ts new file mode 100644 index 00000000000..331d472ad0a --- /dev/null +++ b/apps/sim/scripts/block-registry-snapshot.test.ts @@ -0,0 +1,133 @@ +/** @vitest-environment node */ +import { execFileSync } from 'node:child_process' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { readBlockRegistryAtRef } from '@/scripts/block-registry-snapshot' + +let root: string + +function write(path: string, content: string) { + const target = join(root, path) + mkdirSync(dirname(target), { recursive: true }) + writeFileSync(target, content) +} + +function git(...args: string[]) { + return execFileSync('git', args, { cwd: root, encoding: 'utf8', stdio: 'pipe' }).trim() +} + +function commit() { + git('add', '.') + git( + '-c', + 'user.name=Test', + '-c', + 'commit.gpgsign=false', + '-c', + 'core.hooksPath=/dev/null', + '-c', + 'user.email=test@example.test', + 'commit', + '-m', + 'Baseline fixture' + ) + return git('rev-parse', 'HEAD') +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'registry-snapshot-test-')) + git('init', '--quiet') + write('.gitignore', 'node_modules\n') + write('apps/sim/package.json', JSON.stringify({ name: '@sim/app', type: 'module' })) + write( + 'apps/sim/tsconfig.json', + JSON.stringify({ compilerOptions: { paths: { '@/*': ['./*'] } } }) + ) +}) + +afterEach(() => rmSync(root, { recursive: true, force: true })) + +describe('readBlockRegistryAtRef', () => { + it('reads effective IDs from spreads, local arrays, helpers, and derived blocks at the base revision', () => { + write( + 'apps/sim/blocks/registry.ts', + ` +import { sharedFields } from '@sim/fields' +import { triggerFields } from '@/triggers/fields' +const localFields = [{ id: 'operation', options: [{ id: 'nested-option' }] }, { id: 'encoding' }] +const LegacyBlock = { type: 'legacy', subBlocks: localFields } satisfies { type: string; subBlocks: { id: string }[] } +const CurrentBlock = { ...LegacyBlock, type: 'current', subBlocks: LegacyBlock.subBlocks.filter(field => field.id !== 'encoding') } +const makeFields = () => [...sharedFields, ...triggerFields] +const SpreadBlock = { type: 'spread', subBlocks: [...localFields, ...makeFields()] } +export const getBlockRegistry = () => ({ legacy: LegacyBlock, current: CurrentBlock, spread: SpreadBlock }) +` + ) + write('apps/sim/triggers/fields.ts', "export const triggerFields = [{ id: 'trigger' }]\n") + write( + 'packages/fields/package.json', + JSON.stringify({ name: '@sim/fields', type: 'module', exports: './index.ts' }) + ) + write('packages/fields/index.ts', "export const sharedFields = [{ id: 'shared' }]\n") + const base = commit() + mkdirSync(join(root, 'node_modules/@sim'), { recursive: true }) + symlinkSync(join(root, 'packages/fields'), join(root, 'node_modules/@sim/fields'), 'dir') + write( + 'packages/fields/index.ts', + "export const sharedFields = [{ id: 'changed-after-base' }]\n" + ) + write('apps/sim/triggers/fields.ts', 'export const triggerFields = []\n') + const statusBefore = git('status', '--porcelain') + + expect(readBlockRegistryAtRef(root, base)).toEqual({ + legacy: ['operation', 'encoding'], + current: ['operation'], + spread: ['operation', 'encoding', 'shared', 'trigger'], + }) + expect(git('status', '--porcelain')).toBe(statusBefore) + expect(readFileSync(join(root, 'packages/fields/index.ts'), 'utf8')).toContain( + 'changed-after-base' + ) + }) + + it('keeps installed third-party dependencies available without treating their output as registry JSON', () => { + write( + 'apps/sim/blocks/registry.ts', + ` +import { field } from 'fixture-provider' +console.log('Registry initialization diagnostic') +export const getBlockRegistry = () => ({ block: { type: 'block', subBlocks: [field] } }) +` + ) + const base = commit() + write( + 'node_modules/fixture-provider/package.json', + JSON.stringify({ name: 'fixture-provider', type: 'module', exports: './index.js' }) + ) + write( + 'node_modules/fixture-provider/index.js', + "export const field = { id: 'installed-field' }\n" + ) + + expect(readBlockRegistryAtRef(root, base)).toEqual({ block: ['installed-field'] }) + }) + + it('fails instead of returning partial IDs when a derived definition cannot load', () => { + write( + 'apps/sim/blocks/registry.ts', + ` +import { missingFields } from './missing' +export const getBlockRegistry = () => ({ block: { type: 'block', subBlocks: missingFields } }) +` + ) + const base = commit() + expect(() => readBlockRegistryAtRef(root, base)).toThrow() + }) + + it('fails when the requested base revision is unavailable', () => { + write('apps/sim/blocks/registry.ts', 'export const getBlockRegistry = () => ({})\n') + commit() + expect(() => readBlockRegistryAtRef(root, 'missing-base')).toThrow() + }) +}) diff --git a/apps/sim/scripts/block-registry-snapshot.ts b/apps/sim/scripts/block-registry-snapshot.ts new file mode 100644 index 00000000000..0fa0d9de387 --- /dev/null +++ b/apps/sim/scripts/block-registry-snapshot.ts @@ -0,0 +1,134 @@ +import { execFileSync } from 'node:child_process' +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join, resolve, sep } from 'node:path' +import { z } from 'zod' + +const registryIdsSchema = z.record(z.string().min(1), z.array(z.string().min(1))) + +interface WorkspacePackage { + name: string + path: string + relativePath: string +} + +function readWorkspacePackages(snapshot: string): WorkspacePackage[] { + const workspaces: WorkspacePackage[] = [] + for (const group of ['apps', 'packages']) { + const directory = join(snapshot, group) + if (!existsSync(directory)) continue + for (const entry of readdirSync(directory, { withFileTypes: true })) { + if (!entry.isDirectory()) continue + const relativePath = join(group, entry.name) + const path = join(snapshot, relativePath) + const manifestPath = join(path, 'package.json') + if (!existsSync(manifestPath)) continue + const { name } = z + .object({ name: z.string().min(1) }) + .parse(JSON.parse(readFileSync(manifestPath, 'utf8'))) + workspaces.push({ name, path, relativePath }) + } + } + return workspaces +} + +function linkInstalledDependencies( + source: string, + target: string, + root: string, + workspaceNames: Set, + scope = '' +) { + if (!existsSync(source)) return + mkdirSync(target, { recursive: true }) + for (const entry of readdirSync(source, { withFileTypes: true })) { + const sourcePath = join(source, entry.name) + const targetPath = join(target, entry.name) + if (entry.name.startsWith('@')) { + linkInstalledDependencies(sourcePath, targetPath, root, workspaceNames, `${entry.name}/`) + continue + } + if (workspaceNames.has(scope + entry.name)) continue + const resolved = realpathSync(sourcePath) + if (['apps', 'packages'].some((group) => resolved.startsWith(join(root, group) + sep))) continue + symlinkSync(sourcePath, targetPath, 'dir') + } +} + +function linkWorkspaceDependencies(root: string, snapshot: string) { + const workspaces = readWorkspacePackages(snapshot) + const workspaceNames = new Set(workspaces.map(({ name }) => name)) + const modules = join(snapshot, 'node_modules') + linkInstalledDependencies(join(root, 'node_modules'), modules, root, workspaceNames) + for (const workspace of workspaces) { + const packageLink = resolve(modules, workspace.name) + if (!packageLink.startsWith(modules + sep)) { + throw new Error(`Invalid workspace package name: ${workspace.name}`) + } + mkdirSync(dirname(packageLink), { recursive: true }) + symlinkSync(workspace.path, packageLink, 'dir') + linkInstalledDependencies( + join(root, workspace.relativePath, 'node_modules'), + join(workspace.path, 'node_modules'), + root, + workspaceNames + ) + } +} + +/** + * Reads effective subblock IDs from the base revision's complete source tree. + * Workspace packages resolve inside the snapshot; only installed third-party + * dependencies are shared. A failed import or missing revision fails the audit. + */ +export function readBlockRegistryAtRef(root: string, ref: string): Record { + root = realpathSync(root) + const gitOptions = { cwd: root, encoding: 'utf8' as const, stdio: 'pipe' as const } + const commit = execFileSync( + 'git', + ['rev-parse', '--verify', '--end-of-options', `${ref}^{commit}`], + gitOptions + ).trim() + const temporary = mkdtempSync(join(tmpdir(), 'sim-block-registry-')) + try { + const archive = join(temporary, 'source.tar') + const snapshot = join(temporary, 'source') + mkdirSync(snapshot) + execFileSync('git', ['archive', '--format=tar', `--output=${archive}`, commit], gitOptions) + execFileSync('tar', ['-xf', archive, '-C', snapshot]) + linkWorkspaceDependencies(root, snapshot) + + const script = join(snapshot, 'apps/sim/.block-registry-snapshot.ts') + const output = join(temporary, 'ids.json') + writeFileSync( + script, + ` +import { writeFileSync } from 'node:fs' +import { getBlockRegistry } from '@/blocks/registry' + +const entries = Object.values(getBlockRegistry()).map(block => [block.type, block.subBlocks.map(field => field.id)]) +writeFileSync(process.argv[2], JSON.stringify(Object.fromEntries(entries))) +` + ) + execFileSync('bun', ['--no-env-file', 'run', script, output], { + cwd: join(snapshot, 'apps/sim'), + encoding: 'utf8', + stdio: 'pipe', + timeout: 60_000, + maxBuffer: 4 * 1024 * 1024, + }) + return registryIdsSchema.parse(JSON.parse(readFileSync(output, 'utf8'))) + } finally { + rmSync(temporary, { recursive: true, force: true }) + } +} diff --git a/apps/sim/scripts/block-registry-source.test.ts b/apps/sim/scripts/block-registry-source.test.ts deleted file mode 100644 index d8fc55680b2..00000000000 --- a/apps/sim/scripts/block-registry-source.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { extractInlineBlockSubBlockIds } from '@/scripts/block-registry-source' - -describe('extractInlineBlockSubBlockIds', () => { - it('does not attribute a satisfies-style legacy block’s fields to a typed successor', () => { - expect( - extractInlineBlockSubBlockIds(` - export const LegacyBlock = { - type: 'legacy', - subBlocks: [{ id: 'operation' }, { id: 'encoding' }], - } satisfies BlockConfig - export const CurrentBlock: BlockConfig = { - ...LegacyBlock, - type: 'current', - subBlocks: LegacyBlock.subBlocks.filter((field) => field.id !== 'encoding'), - } - `) - ).toEqual({ legacy: ['operation', 'encoding'] }) - }) - - it('reads each inline definition and ignores nested option IDs and punctuation in strings', () => { - expect( - extractInlineBlockSubBlockIds(` - export const FirstBlock: BlockConfig = { - type: 'first', subBlocks: [{ id: 'operation', options: [{ id: 'nested' }] }], - } - export const SecondBlock: BlockConfig = { - type: 'second', subBlocks: [ - { id: 'template', placeholder: 'objects: [{ }]' }, - { id: 'text' }, - ], - } - `) - ).toEqual({ first: ['operation'], second: ['template', 'text'] }) - }) -}) diff --git a/apps/sim/scripts/block-registry-source.ts b/apps/sim/scripts/block-registry-source.ts deleted file mode 100644 index 2fb155df7e6..00000000000 --- a/apps/sim/scripts/block-registry-source.ts +++ /dev/null @@ -1,51 +0,0 @@ -import ts from '@typescript/typescript6' - -function unwrap(expression: ts.Expression): ts.Expression { - while ( - ts.isSatisfiesExpression(expression) || - ts.isAsExpression(expression) || - ts.isParenthesizedExpression(expression) - ) { - expression = expression.expression - } - return expression -} - -function property(object: ts.ObjectLiteralExpression, name: string): ts.Expression | undefined { - const member = object.properties.find( - (member) => - ts.isPropertyAssignment(member) && - (ts.isIdentifier(member.name) || ts.isStringLiteral(member.name)) && - member.name.text === name - ) - return member && ts.isPropertyAssignment(member) ? member.initializer : undefined -} - -/** Extracts inline subblock IDs from their owning declaration, including satisfies-style blocks. */ -export function extractInlineBlockSubBlockIds(source: string): Record { - const file = ts.createSourceFile('block.ts', source, ts.ScriptTarget.Latest, true) - const blocks: Record = {} - for (const statement of file.statements) { - if (!ts.isVariableStatement(statement)) continue - for (const declaration of statement.declarationList.declarations) { - if (!declaration.initializer) continue - const object = unwrap(declaration.initializer) - if (!ts.isObjectLiteralExpression(object)) continue - const type = property(object, 'type') - const subBlocks = property(object, 'subBlocks') - if ( - !type || - !ts.isStringLiteral(type) || - !subBlocks || - !ts.isArrayLiteralExpression(subBlocks) - ) - continue - blocks[type.text] = subBlocks.elements.flatMap((element) => { - if (!ts.isObjectLiteralExpression(element)) return [] - const id = property(element, 'id') - return id && ts.isStringLiteral(id) ? [id.text] : [] - }) - } - } - return blocks -} diff --git a/apps/sim/scripts/check-block-registry.ts b/apps/sim/scripts/check-block-registry.ts index de69213768a..8c453be49ca 100644 --- a/apps/sim/scripts/check-block-registry.ts +++ b/apps/sim/scripts/check-block-registry.ts @@ -24,76 +24,37 @@ * bun run apps/sim/scripts/check-block-registry.ts origin/main */ -import { execSync } from 'child_process' +import { execFileSync } from 'node:child_process' import { SUBBLOCK_ID_MIGRATIONS } from '@/lib/workflows/migrations/subblock-migrations' -import { getAllBlocks, getBlock, getBlockMeta } from '@/blocks/registry' -import { extractInlineBlockSubBlockIds } from '@/scripts/block-registry-source' +import { getAllBlocks, getBlock, getBlockMeta, getBlockRegistry } from '@/blocks/registry' +import { readBlockRegistryAtRef } from '@/scripts/block-registry-snapshot' import { getToolParams } from '@/tools/metadata' const baseRef = process.argv[2] || 'HEAD~1' -const gitRoot = execSync('git rev-parse --show-toplevel', { encoding: 'utf-8' }).trim() +const gitRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf-8' }).trim() const gitOpts = { encoding: 'utf-8' as const, cwd: gitRoot } type IdMap = Record> function getCurrentIds(): IdMap { const map: IdMap = {} - for (const block of getAllBlocks()) { + for (const block of Object.values(getBlockRegistry())) { map[block.type] = new Set(block.subBlocks.map((sb) => sb.id)) } return map } -type PreviousIdsResult = - | { kind: 'skip'; reason: string } - | { kind: 'noop' } - | { kind: 'ok'; map: IdMap } - -function getPreviousIds(): PreviousIdsResult { - const registryPath = 'apps/sim/blocks/registry.ts' - const blocksDir = 'apps/sim/blocks/blocks' - - let hasChanges = false - try { - const diff = execSync( - `git diff --name-only ${baseRef} -- ${registryPath} ${blocksDir}`, - gitOpts - ).trim() - hasChanges = diff.length > 0 - } catch { - return { kind: 'skip', reason: 'Could not diff against base ref' } - } - - if (!hasChanges) { - return { kind: 'noop' } - } +function getPreviousIds(): IdMap | null { + const changed = execFileSync( + 'git', + ['diff', '--name-only', baseRef, '--', 'apps/sim/blocks', 'apps/sim/triggers'], + gitOpts + ).trim() + if (!changed) return null - const map: IdMap = {} - - try { - const blockFiles = execSync(`git ls-tree -r --name-only ${baseRef} -- ${blocksDir}`, gitOpts) - .trim() - .split('\n') - .filter((f) => f.endsWith('.ts') && !f.endsWith('.test.ts')) - - for (const filePath of blockFiles) { - let content: string - try { - content = execSync(`git show ${baseRef}:${filePath}`, gitOpts) - } catch { - continue - } - - for (const [blockType, ids] of Object.entries(extractInlineBlockSubBlockIds(content))) { - if (ids.length > 0) map[blockType] = new Set(ids) - } - } - } catch (err) { - return { kind: 'skip', reason: `Could not read previous block files from ${baseRef}: ${err}` } - } - - return { kind: 'ok', map } + const previous = readBlockRegistryAtRef(gitRoot, baseRef) + return Object.fromEntries(Object.entries(previous).map(([type, ids]) => [type, new Set(ids)])) } type CheckResult = @@ -104,10 +65,7 @@ type CheckResult = function checkSubblockIdStability(): CheckResult { const previous = getPreviousIds() - if (previous.kind === 'skip') { - return { kind: 'skip', message: `${previous.reason} — skipping subblock ID stability check` } - } - if (previous.kind === 'noop') { + if (previous === null) { return { kind: 'skip', message: 'No block definition changes detected — skipping subblock ID stability check', @@ -117,7 +75,7 @@ function checkSubblockIdStability(): CheckResult { const current = getCurrentIds() const errors: string[] = [] - for (const [blockType, prevIds] of Object.entries(previous.map)) { + for (const [blockType, prevIds] of Object.entries(previous)) { const currIds = current[blockType] if (!currIds) continue