diff --git a/.github/workflows/publish-sim-cli.yml b/.github/workflows/publish-sim-cli.yml index 53418f763d4..852423f1231 100644 --- a/.github/workflows/publish-sim-cli.yml +++ b/.github/workflows/publish-sim-cli.yml @@ -15,6 +15,13 @@ concurrency: jobs: publish-npm: + # Job-level, not on the build step: `bun publish` runs `prepublishOnly`, + # which rebuilds `dist` a second time, and that second build is the one + # that ships. A build without the token reports nothing. See + # docs/cli/usage-data. + env: + SIM_CLI_TELEMETRY_KEY: ${{ vars.SIM_CLI_TELEMETRY_KEY }} + SIM_CLI_TELEMETRY_HOST: ${{ vars.SIM_CLI_TELEMETRY_HOST }} runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-4vcpu-ubuntu-2404' || 'ubuntu-latest' }} timeout-minutes: 15 steps: @@ -63,11 +70,6 @@ jobs: - name: Build package working-directory: packages/sim-cli - env: - # Public PostHog project token for anonymous CLI usage reporting; a - # build without it reports nothing. See docs/cli/usage-data. - SIM_CLI_TELEMETRY_KEY: ${{ vars.SIM_CLI_TELEMETRY_KEY }} - SIM_CLI_TELEMETRY_HOST: ${{ vars.SIM_CLI_TELEMETRY_HOST }} run: bun run build - name: Resolve release channel diff --git a/apps/docs/content/docs/search/confluence.mdx b/apps/docs/content/docs/search/confluence.mdx index 1f7887a2de1..d6f5b3f806b 100644 --- a/apps/docs/content/docs/search/confluence.mdx +++ b/apps/docs/content/docs/search/confluence.mdx @@ -132,7 +132,7 @@ Central sources combine space permissions, page and ancestor restrictions, and g Open **Settings → Sources → Confluence**, then a source's **Documents**, **Settings**, or **Sync history**. Invite teammates through **Settings → Members → Invite** or SSO, then have them connect through **Integrations**. **People → Request connections** only requests a provider connection; it does not invite people to the organization. -Syncing runs automatically. Admins can use **Sync now** for an immediate update, **Pause syncing** to stop scheduled syncs, or **Resume syncing** to restart them. **Full resync**, available for service-account connections, fetches unchanged content again and asks for confirmation. Successful manual syncs have a one-minute cooldown; failed syncs can be retried immediately. +Syncing runs automatically. Admins can use **Sync now** for an immediate update, **Pause syncing** to stop scheduled syncs, or **Resume syncing** to restart them. Successful manual syncs have a one-minute cooldown; failed syncs can be retried immediately. ## Troubleshooting diff --git a/apps/docs/content/docs/search/connect-your-account.mdx b/apps/docs/content/docs/search/connect-your-account.mdx index 353fc520956..b82cdf817a2 100644 --- a/apps/docs/content/docs/search/connect-your-account.mdx +++ b/apps/docs/content/docs/search/connect-your-account.mdx @@ -78,7 +78,7 @@ On the main **Integrations** page, select **Reconnect** beside the integration i Admins manage setup from **Settings → Sources**. Open an integration, then its connection to see **Documents**, **Settings**, and **Sync history**. **People** shows account contributors across integrations and supports filtering by integration. This does not grant the admin access to every document. -Syncing runs automatically. Admins can use **Sync now** when they need an update; another manual run is available 60 seconds after a successful sync finishes. Failed or partial runs can be retried immediately. The connection header also offers **Pause syncing** or **Resume syncing**, and **Remove connection**. Where supported, **Full resync** fetches and reindexes all content and asks for confirmation first. +Syncing runs automatically. Admins can use **Sync now** when they need an update; another manual run is available 60 seconds after a successful sync finishes. Failed or partial runs can be retried immediately. The connection header also offers **Pause syncing** or **Resume syncing**, and **Remove connection**. Removing a Search connection also removes its indexed documents from Sim. The originals remain in the connected app. diff --git a/apps/docs/content/docs/search/gitlab.mdx b/apps/docs/content/docs/search/gitlab.mdx index 7dbc4f15fa5..55848547f6c 100644 --- a/apps/docs/content/docs/search/gitlab.mdx +++ b/apps/docs/content/docs/search/gitlab.mdx @@ -146,7 +146,7 @@ Open a project to use these administrator actions: | **Settings** | Change the token, project, filters, or CSV permissions. | | **Remove connection** | Confirm removal of the connection and its indexed documents. Documents cannot be retained without the connection that maintains their permissions. | -GitLab does not expose a separate **Full resync** action. Each sync checks the selected content. CSV grants change only when you replace the files. +Each sync checks the selected content. CSV grants change only when you replace the files. ## Troubleshooting diff --git a/apps/docs/content/docs/search/index.mdx b/apps/docs/content/docs/search/index.mdx index 68b53e20f2a..063d27d9243 100644 --- a/apps/docs/content/docs/search/index.mdx +++ b/apps/docs/content/docs/search/index.mdx @@ -104,7 +104,7 @@ Edit **Settings** to change an existing connection's filters. Adding another con **Sync using** shows the method selected when the source was created. Add a new connection to change that method. To replace a supported indexing credential, select its replacement and use **Change service account** or **Change account**, as shown. -The connection header offers **Sync now**, **Pause syncing** or **Resume syncing**, **Remove connection**, and, where supported, **Full resync**. Full resync fetches all content again and requires confirmation. Manual runs have a 60-second cooldown after a successful sync finishes; failed or partial runs can be retried immediately. **Pause syncing** becomes available when the current sync finishes. +The connection header offers **Sync now**, **Pause syncing** or **Resume syncing**, and **Remove connection**. Manual runs have a 60-second cooldown after a successful sync finishes; failed or partial runs can be retried immediately. **Pause syncing** becomes available when the current sync finishes. To deactivate an entire integration, open it from **Settings → Sources**, select **Deactivate**, and confirm. Its content becomes unavailable in Search, Assistant, and MCP; saved connections remain. Select **Activate** on that integration to enable it again. diff --git a/apps/realtime/src/database/operations.test.ts b/apps/realtime/src/database/operations.test.ts new file mode 100644 index 00000000000..fb81bbd304d --- /dev/null +++ b/apps/realtime/src/database/operations.test.ts @@ -0,0 +1,171 @@ +/** @vitest-environment node */ +import { OPERATION_TARGETS, SUBBLOCK_OPERATIONS } from '@sim/realtime-protocol/constants' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockTransaction, mockSelectWhere, mockSet } = vi.hoisted(() => ({ + mockTransaction: vi.fn(), + mockSelectWhere: vi.fn(), + mockSet: vi.fn(), +})) + +vi.mock('@sim/audit', () => ({ AuditAction: {}, AuditResourceType: {}, recordAudit: vi.fn() })) +vi.mock('@sim/db', () => ({ + instrumentPoolClient: vi.fn(), + resolveDbUrl: vi.fn(() => 'postgres://localhost/test'), + workflow: { id: 'workflow.id' }, + workflowBlocks: { id: 'block.id', workflowId: 'block.workflowId' }, + workflowEdges: {}, + workflowSubflows: {}, +})) +vi.mock('@sim/db/timestamps', () => ({ withUtcTimestamps: (options: unknown) => options })) +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }), +})) +vi.mock('@sim/platform-authz/workflow', () => ({ + getActiveWorkflowContext: vi.fn().mockResolvedValue({ id: 'workflow-1' }), +})) +vi.mock('@sim/workflow-persistence/load', () => ({ + loadWorkflowFromNormalizedTablesRaw: vi.fn(), +})) +vi.mock('@sim/workflow-persistence/subblocks', () => ({ mergeSubBlockValues: vi.fn() })) +vi.mock('drizzle-orm', () => ({ + and: vi.fn(), + eq: vi.fn(), + inArray: vi.fn(), + isNull: vi.fn(), + or: vi.fn(), + sql: vi.fn(), +})) +vi.mock('drizzle-orm/postgres-js', () => ({ drizzle: () => ({ transaction: mockTransaction }) })) +vi.mock('postgres', () => ({ default: vi.fn() })) +vi.mock('@/env', () => ({ + env: { DATABASE_URL: 'postgres://localhost/test' }, +})) + +import { persistWorkflowOperation } from '@/database/operations' + +const transaction = { + select: () => ({ from: () => ({ where: mockSelectWhere }) }), + update: () => ({ set: mockSet }), + delete: vi.fn(), + insert: vi.fn(), +} + +describe('search replacement persistence', () => { + const expected = [ + { + type: 'function', + params: { language: 'javascript', code: 'return 1' }, + usageControl: 'none', + }, + ] + const replacement = [{ ...expected[0], params: { ...expected[0].params, code: 'return 2' } }] + + beforeEach(() => { + vi.clearAllMocks() + mockTransaction.mockImplementation( + async (callback: (tx: typeof transaction) => Promise) => callback(transaction) + ) + mockSet.mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }) + }) + + function replaceTools(stored: unknown, expectedValue: unknown = expected) { + mockSelectWhere.mockResolvedValue([ + { + id: 'agent-1', + type: 'agent', + locked: false, + data: {}, + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: stored } }, + }, + ]) + return persistWorkflowOperation('workflow-1', { + operation: SUBBLOCK_OPERATIONS.BATCH_UPDATE, + target: OPERATION_TARGETS.SUBBLOCK, + timestamp: Date.now(), + payload: { + updates: [{ blockId: 'agent-1', subblockId: 'tools', value: replacement, expectedValue }], + }, + }) + } + + it('accepts equivalent nested tool objects after JSONB changes their key order', async () => { + const stored = [ + { + usageControl: 'none', + params: { code: 'return 1', language: 'javascript' }, + type: 'function', + }, + ] + + await expect(replaceTools(stored)).resolves.toBeUndefined() + expect(mockSet).toHaveBeenLastCalledWith( + expect.objectContaining({ + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: replacement } }, + }) + ) + }) + + it('still rejects a tool parameter changed by another editor', async () => { + await expect( + replaceTools([{ ...expected[0], params: { ...expected[0].params, code: 'return 3' } }]) + ).rejects.toThrow('changed since replacement was planned') + expect(mockSet).toHaveBeenCalledTimes(1) + }) + + it('still rejects reordered tool arrays', async () => { + const another = { ...expected[0], params: { ...expected[0].params, code: 'return 3' } } + await expect(replaceTools([another, expected[0]], [expected[0], another])).rejects.toThrow( + 'changed since replacement was planned' + ) + expect(mockSet).toHaveBeenCalledTimes(1) + }) +}) + +describe('subblock update with canonical modes persistence', () => { + const tools = [{ type: 'jira', params: { manualProjectId: '{{PROJECT}}' } }] + const canonicalModes = { '0:projectId': 'advanced' as const, model: 'basic' as const } + + beforeEach(() => { + vi.clearAllMocks() + mockTransaction.mockImplementation( + async (callback: (tx: typeof transaction) => Promise) => callback(transaction) + ) + mockSet.mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }) + }) + + function updateTools(block: Record) { + mockSelectWhere.mockResolvedValue([ + { + id: 'agent-1', + locked: false, + data: { width: 350, canonicalModes: { '1:projectId': 'advanced' } }, + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: [] } }, + ...block, + }, + ]) + return persistWorkflowOperation('workflow-1', { + operation: SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES, + target: OPERATION_TARGETS.SUBBLOCK, + timestamp: Date.now(), + payload: { blockId: 'agent-1', subblockId: 'tools', value: tools, canonicalModes }, + }) + } + + it('writes the subblock value and replaces canonical modes in one block update', async () => { + await expect(updateTools({})).resolves.toBeUndefined() + + expect(mockSet).toHaveBeenCalledTimes(2) + expect(mockSet).toHaveBeenLastCalledWith( + expect.objectContaining({ + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: tools } }, + data: { width: 350, canonicalModes }, + }) + ) + }) + + it('rejects a locked block without writing either field', async () => { + await expect(updateTools({ locked: true })).rejects.toThrow('is locked') + expect(mockSet).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/realtime/src/database/operations.ts b/apps/realtime/src/database/operations.ts index f28543a8004..cd4316448bf 100644 --- a/apps/realtime/src/database/operations.ts +++ b/apps/realtime/src/database/operations.ts @@ -1,3 +1,4 @@ +import { isDeepStrictEqual } from 'node:util' import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import * as schema from '@sim/db' import { @@ -25,6 +26,7 @@ import { import { randomFloat } from '@sim/utils/random' import { loadWorkflowFromNormalizedTablesRaw } from '@sim/workflow-persistence/load' import { mergeSubBlockValues } from '@sim/workflow-persistence/subblocks' +import type { DbOrTx } from '@sim/workflow-persistence/types' import { filterAcyclicEdges, filterUniqueWorkflowEdges, @@ -1988,8 +1990,36 @@ async function handleSubflowOperationTx( } } -function valuesEqual(left: unknown, right: unknown): boolean { - return JSON.stringify(left) === JSON.stringify(right) +/** Every block in the workflow by id, for the locked-container check subblock writes need. */ +async function loadSubblockUpdateBlocks(tx: DbOrTx, workflowId: string) { + const allBlocks = await tx + .select({ + id: workflowBlocks.id, + subBlocks: workflowBlocks.subBlocks, + locked: workflowBlocks.locked, + data: workflowBlocks.data, + }) + .from(workflowBlocks) + .where(eq(workflowBlocks.workflowId, workflowId)) + return Object.fromEntries(allBlocks.map((block) => [block.id, block])) +} + +/** + * The block a subblock write targets, rejecting one that is missing, locked, or in a locked + * container. + */ +function getWritableSubblockUpdateBlock( + blocksById: Awaited>, + blockId: string +) { + const block = blocksById[blockId] + if (!block) { + throw new Error(`Block ${blockId} not found`) + } + if (isWorkflowBlockProtected(blockId, blocksById)) { + throw new Error(`Block ${blockId} is locked or inside a locked container`) + } + return block } // Subblock operations - targeted value updates without replacing workflow state @@ -2006,20 +2036,7 @@ async function handleSubblockOperationTx( return } - const allBlocks = await tx - .select({ - id: workflowBlocks.id, - subBlocks: workflowBlocks.subBlocks, - locked: workflowBlocks.locked, - data: workflowBlocks.data, - }) - .from(workflowBlocks) - .where(eq(workflowBlocks.workflowId, workflowId)) - - type SubblockUpdateBlockRecord = (typeof allBlocks)[number] - const blocksById: Record = Object.fromEntries( - allBlocks.map((block: SubblockUpdateBlockRecord) => [block.id, block]) - ) + const blocksById = await loadSubblockUpdateBlocks(tx, workflowId) for (const update of updates) { const { blockId, subblockId, value, expectedValue } = update @@ -2027,19 +2044,13 @@ async function handleSubblockOperationTx( throw new Error('Missing required fields for subblock batch update') } - const block = blocksById[blockId] - if (!block) { - throw new Error(`Block ${blockId} not found`) - } - - if (isWorkflowBlockProtected(blockId, blocksById)) { - throw new Error(`Block ${blockId} is locked or inside a locked container`) - } + const block = getWritableSubblockUpdateBlock(blocksById, blockId) const subBlocks = { ...((block.subBlocks as Record) || {}) } const currentSubBlock = subBlocks[subblockId] const currentValue = currentSubBlock?.value - if (expectedValue !== undefined && !valuesEqual(currentValue, expectedValue)) { + /** JSONB can reorder object keys; changed values and array order must still conflict. */ + if (expectedValue !== undefined && !isDeepStrictEqual(currentValue, expectedValue)) { throw new Error(`Subblock ${blockId}.${subblockId} changed since replacement was planned`) } @@ -2062,6 +2073,36 @@ async function handleSubblockOperationTx( break } + case SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES: { + const { blockId, subblockId, value, canonicalModes } = payload + if (!blockId || !subblockId || !canonicalModes) { + throw new Error('Missing required fields for subblock update with canonical modes') + } + + const blocksById = await loadSubblockUpdateBlocks(tx, workflowId) + const block = getWritableSubblockUpdateBlock(blocksById, blockId) + + const subBlocks = { + ...((block.subBlocks as Record> | null) || {}), + } + const currentSubBlock = subBlocks[subblockId] + subBlocks[subblockId] = currentSubBlock + ? { ...currentSubBlock, value } + : { id: subblockId, type: 'unknown', value } + + await tx + .update(workflowBlocks) + .set({ + subBlocks, + data: { ...((block.data as Record) || {}), canonicalModes }, + updatedAt: new Date(), + }) + .where(and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId))) + + logger.debug(`Updated subblock ${blockId}.${subblockId} with canonical modes`) + break + } + default: logger.warn(`Unknown subblock operation: ${operation}`) throw new Error(`Unsupported subblock operation: ${operation}`) diff --git a/apps/realtime/src/middleware/permissions.test.ts b/apps/realtime/src/middleware/permissions.test.ts index 4edc05d795a..97f32b1d2d2 100644 --- a/apps/realtime/src/middleware/permissions.test.ts +++ b/apps/realtime/src/middleware/permissions.test.ts @@ -114,6 +114,11 @@ describe('checkRolePermission', () => { const result = checkRolePermission('write', 'subblock-batch-update') expectPermissionAllowed(result) }) + + it('should allow subblock-update-with-canonical-modes operation', () => { + const result = checkRolePermission('write', 'subblock-update-with-canonical-modes') + expectPermissionAllowed(result) + }) }) describe('read role', () => { @@ -155,6 +160,11 @@ describe('checkRolePermission', () => { expectPermissionDenied(result, 'read') }) + it('should deny subblock-update-with-canonical-modes operation for read role', () => { + const result = checkRolePermission('read', 'subblock-update-with-canonical-modes') + expectPermissionDenied(result, 'read') + }) + it('should deny toggle-enabled operation for read role', () => { const result = checkRolePermission('read', 'toggle-enabled') expectPermissionDenied(result, 'read') diff --git a/apps/realtime/src/middleware/permissions.ts b/apps/realtime/src/middleware/permissions.ts index e17678461a6..a007de6c8b2 100644 --- a/apps/realtime/src/middleware/permissions.ts +++ b/apps/realtime/src/middleware/permissions.ts @@ -52,6 +52,7 @@ const WRITE_OPERATIONS: string[] = [ // Subblock operations SUBBLOCK_OPERATIONS.UPDATE, SUBBLOCK_OPERATIONS.BATCH_UPDATE, + SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES, // Variable operations VARIABLE_OPERATIONS.UPDATE, // Workflow operations diff --git a/apps/sim/app/(landing)/components/hero/components/hero-chat-loop/hero-tool-call-item.tsx b/apps/sim/app/(landing)/components/hero/components/hero-chat-loop/hero-tool-call-item.tsx index b659eae8b45..068a27599da 100644 --- a/apps/sim/app/(landing)/components/hero/components/hero-chat-loop/hero-tool-call-item.tsx +++ b/apps/sim/app/(landing)/components/hero/components/hero-chat-loop/hero-tool-call-item.tsx @@ -2,7 +2,10 @@ import { Table } from '@sim/emcn/icons' import { SlackIcon } from '@/components/icons' import { ActivityStatus } from '@/components/ui/activity-status' import { getToolStatusDisplayTitle } from '@/lib/copilot/tools/tool-display' -import type { ToolCallItemProps } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item' +import type { + ToolActivityPresentation, + ToolCallItemProps, +} from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item' import { getToolIcon } from '@/app/workspace/[workspaceId]/home/components/message-content/utils' /** Demo fixtures have known brands, so the landing page never loads the block registry. */ @@ -11,6 +14,7 @@ export function HeroToolCallItem({ renderStatus, toolName, displayTitle, + activityDescription, status, }: ToolCallItemProps) { const Icon = @@ -19,12 +23,16 @@ export function HeroToolCallItem({ : toolCallId === 'hero-read-table' ? Table : getToolIcon(toolName) - const activity = ( - } - /> - ) - return renderStatus ? renderStatus(activity) : activity + const activity: ToolActivityPresentation = { + label: getToolStatusDisplayTitle(displayTitle, status, toolName, activityDescription), + activeLabel: getToolStatusDisplayTitle( + displayTitle, + status === 'success' ? 'executing' : status, + toolName, + activityDescription + ), + isActive: status === 'executing', + icon: , + } + return renderStatus ? renderStatus(activity) : } diff --git a/apps/sim/app/api/auth/sso/register/route.test.ts b/apps/sim/app/api/auth/sso/register/route.test.ts index b4c4ef1704f..db55de96da8 100644 --- a/apps/sim/app/api/auth/sso/register/route.test.ts +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -88,6 +88,7 @@ const OIDC_BODY = { providerId: 'acme-oidc', issuer: 'https://idp.acme.com', domain: 'acme.com', + orgId: 'org1', clientId: 'client-id', clientSecret: 'client-secret', authorizationEndpoint: 'https://idp.acme.com/authorize', @@ -137,21 +138,21 @@ describe('POST /api/auth/sso/register', () => { it('rejects callers without an Enterprise plan', async () => { mockHasSSOAccess.mockResolvedValue(false) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(403) expect(mockRegisterSSOProvider).not.toHaveBeenCalled() }) it('rejects callers who are not an admin/owner of the target org', async () => { queueMembers([{ organizationId: 'org1', role: 'member' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(403) expect(mockRegisterSSOProvider).not.toHaveBeenCalled() }) it('rejects an invalid domain', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - const res = await POST(request({ ...OIDC_BODY, domain: 'not-a-domain', orgId: 'org1' })) + const res = await POST(request({ ...OIDC_BODY, domain: 'not-a-domain' })) expect(res.status).toBe(400) expect(mockRegisterSSOProvider).not.toHaveBeenCalled() }) @@ -160,7 +161,7 @@ describe('POST /api/auth/sso/register', () => { resetDbChainMock() queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueTableRows(schemaMock.ssoDomain, []) // no verified sso_domain row - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) const json = await res.json() expect(res.status).toBe(403) expect(json.code).toBe('SSO_DOMAIN_NOT_VERIFIED') @@ -172,7 +173,7 @@ describe('POST /api/auth/sso/register', () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueTableRows(schemaMock.ssoDomain, [{ id: 'v' }]) // entry gate: verified queueTableRows(schemaMock.ssoDomain, []) // re-check before write: revoked - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) const json = await res.json() expect(res.status).toBe(403) expect(json.code).toBe('SSO_DOMAIN_NOT_VERIFIED') @@ -185,7 +186,7 @@ describe('POST /api/auth/sso/register', () => { queueTableRows(schemaMock.ssoDomain, [{ id: 'v' }]) // entry gate: verified queueTableRows(schemaMock.ssoDomain, [{ id: 'v' }]) // pre-write re-check: verified queueTableRows(schemaMock.ssoDomain, []) // locking read in the grant: proof gone - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) const json = await res.json() expect(res.status).toBe(403) expect(json.code).toBe('SSO_DOMAIN_NOT_VERIFIED') @@ -221,7 +222,7 @@ describe('POST /api/auth/sso/register', () => { queueProviders([ { domain: 'acme.com', userId: 'u1', organizationId: 'org1', providerId: 'acme-saml' }, ]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) const json = await res.json() expect(res.status).toBe(409) expect(json.code).toBe('SSO_DOMAIN_ALREADY_ROUTED') @@ -238,7 +239,7 @@ describe('POST /api/auth/sso/register', () => { constraint_name: 'sso_provider_org_domain_unique', }) ) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) const json = await res.json() expect(res.status).toBe(409) expect(json.code).toBe('SSO_DOMAIN_ALREADY_ROUTED') @@ -248,14 +249,14 @@ describe('POST /api/auth/sso/register', () => { it('lets the organization add a provider for a different verified domain', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueProviders([]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1', domain: 'eng.acme.com' })) + const res = await POST(request({ ...OIDC_BODY, domain: 'eng.acme.com' })) expect(res.status).toBe(200) expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) }) it('registers when the domain is unclaimed', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) }) @@ -286,7 +287,7 @@ describe('POST /api/auth/sso/register', () => { it('does not treat the caller’s own provider as a providerId conflict', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueProviders([], [{ domain: 'acme.com', userId: 'u1', organizationId: 'org1' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) }) @@ -298,7 +299,7 @@ describe('POST /api/auth/sso/register', () => { */ it('marks the provider domain-verified after registering', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) expect(dbChainMockFns.set).toHaveBeenCalledWith({ domainVerified: true, @@ -311,7 +312,7 @@ describe('POST /api/auth/sso/register', () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueProviders([]) queueTableRows(schemaMock.ssoProvider, [{ id: 'p1' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) expect(mockUpdateSSOProvider).toHaveBeenCalledTimes(1) expect(dbChainMockFns.set).toHaveBeenCalledWith({ @@ -346,7 +347,7 @@ describe('POST /api/auth/sso/register', () => { }, ]) // provider already owned → update path - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(403) expect(mockUpdateSSOProvider).toHaveBeenCalledTimes(1) // The conditional grant UPDATE is still issued — it simply matches no rows once @@ -376,7 +377,7 @@ describe('POST /api/auth/sso/register', () => { ]) dbChainMockFns.returning.mockRejectedValueOnce(new Error('trust write failed')) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1', jitProvisioningEnabled: false })) + const res = await POST(request({ ...OIDC_BODY, jitProvisioningEnabled: false })) expect(res.status).toBe(500) expect(mockUpdateSSOProvider).toHaveBeenCalledTimes(1) @@ -397,41 +398,28 @@ describe('POST /api/auth/sso/register', () => { queueTableRows(schemaMock.ssoDomain, [{ id: 'verified-domain' }]) queueTableRows(schemaMock.ssoDomain, [{ id: 'verified-domain' }]) queueTableRows(schemaMock.ssoDomain, []) // locking read in the grant: proof gone - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(403) expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) // it was created… expect(dbChainMockFns.delete).toHaveBeenCalled() // …then rolled back }) /** - * A personal provider has no verified domain behind it. On the hosted - * multi-tenant deployment that must grant no linking authority, or anyone able - * to register one could claim a domain they do not own and have their own IdP - * auto-link to existing accounts on it. + * An org-less provider has no `sso_domain` proof behind its domain, and domain + * trust is what auto-links an SSO sign-in into an existing same-email account, + * so the route refuses one before any write. */ - it('does not grant domain trust to a personal provider when hosted', async () => { - setEnvFlags({ isSsoEnabled: true, isHosted: true }) - const res = await POST(request(OIDC_BODY)) - expect(res.status).toBe(200) - expect(dbChainMockFns.set).toHaveBeenCalledWith({ - domainVerified: false, - jitProvisioningEnabled: true, - }) - }) - - it('grants domain trust to a personal provider when self-hosted', async () => { - setEnvFlags({ isSsoEnabled: true, isHosted: false }) - const res = await POST(request(OIDC_BODY)) - expect(res.status).toBe(200) - expect(dbChainMockFns.set).toHaveBeenCalledWith({ - domainVerified: true, - jitProvisioningEnabled: true, - }) + it('refuses a provider without an organization', async () => { + const { orgId: _orgId, ...orgLessBody } = OIDC_BODY + const res = await POST(request(orgLessBody)) + expect(res.status).toBe(400) + expect((await res.json()).error).toContain('Organization ID is required') + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() }) it('persists invite-only provisioning without changing Better Auth provider config', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1', jitProvisioningEnabled: false })) + const res = await POST(request({ ...OIDC_BODY, jitProvisioningEnabled: false })) expect(res.status).toBe(200) expect(dbChainMockFns.set).toHaveBeenCalledWith({ domainVerified: true, @@ -516,9 +504,7 @@ describe('POST /api/auth/sso/register', () => { it('nests the attribute mapping inside oidcConfig (Better Auth reads it there)', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - await POST( - request({ ...OIDC_BODY, orgId: 'org1', mapping: { id: 'oid', email: 'upn', name: 'name' } }) - ) + await POST(request({ ...OIDC_BODY, mapping: { id: 'oid', email: 'upn', name: 'name' } })) expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) const sent = mockRegisterSSOProvider.mock.calls[0][0].body expect(sent.mapping).toBeUndefined() // not passed at the top level (silently ignored there) @@ -529,7 +515,7 @@ describe('POST /api/auth/sso/register', () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueProviders([]) // no providerId or domain conflicts on either pass queueTableRows(schemaMock.ssoProvider, [{ id: 'p1' }]) // provider already owned → edit - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) const body = await res.json() expect(body.message).toContain('updated') @@ -540,15 +526,15 @@ describe('POST /api/auth/sso/register', () => { it('allows the owning tenant to update its own provider for the same domain', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueProviders([{ domain: 'acme.com', userId: 'u1', organizationId: 'org1' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) }) - it('lets an org admin adopt their own user-scoped provider for the same domain', async () => { + it("does not report the caller's own org-less provider as another tenant's claim", async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueProviders([{ domain: 'acme.com', userId: 'u1', organizationId: null }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) }) @@ -556,14 +542,14 @@ describe('POST /api/auth/sso/register', () => { it("still blocks an org admin from claiming another user's user-scoped domain", async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) queueProviders([{ domain: 'acme.com', userId: 'someone-else', organizationId: null }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(409) expect(mockRegisterSSOProvider).not.toHaveBeenCalled() }) it('normalizes the domain before persisting it', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - const res = await POST(request({ ...OIDC_BODY, domain: 'ACME.com', orgId: 'org1' })) + const res = await POST(request({ ...OIDC_BODY, domain: 'ACME.com' })) expect(res.status).toBe(200) expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) const config = mockRegisterSSOProvider.mock.calls[0][0].body @@ -572,7 +558,7 @@ describe('POST /api/auth/sso/register', () => { it('passes skipDiscovery since Sim already resolved and validated the OIDC endpoints', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.skipDiscovery).toBe(true) @@ -580,7 +566,7 @@ describe('POST /api/auth/sso/register', () => { it('omits userInfoEndpoint when skipUserInfoEndpoint is requested, forcing ID token claims', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - const res = await POST(request({ ...OIDC_BODY, skipUserInfoEndpoint: true, orgId: 'org1' })) + const res = await POST(request({ ...OIDC_BODY, skipUserInfoEndpoint: true })) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.userInfoEndpoint).toBeUndefined() @@ -594,7 +580,7 @@ describe('POST /api/auth/sso/register', () => { } return { isValid: true, resolvedIP: '1.2.3.4' } }) - const res = await POST(request({ ...OIDC_BODY, skipUserInfoEndpoint: true, orgId: 'org1' })) + const res = await POST(request({ ...OIDC_BODY, skipUserInfoEndpoint: true })) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.userInfoEndpoint).toBeUndefined() @@ -624,7 +610,7 @@ describe('POST /api/auth/sso/register', () => { jwksEndpoint: undefined, skipUserInfoEndpoint: true, } - const res = await POST(request({ ...discoveredBody, orgId: 'org1' })) + const res = await POST(request(discoveredBody)) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.userInfoEndpoint).toBeUndefined() @@ -632,7 +618,7 @@ describe('POST /api/auth/sso/register', () => { it('keeps userInfoEndpoint when skipUserInfoEndpoint is not requested', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.userInfoEndpoint).toBe('https://idp.acme.com/userinfo') @@ -656,7 +642,7 @@ describe('POST /api/auth/sso/register', () => { tokenEndpoint: undefined, jwksEndpoint: undefined, } - const res = await POST(request({ ...discoveredBody, orgId: 'org1' })) + const res = await POST(request(discoveredBody)) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') @@ -670,7 +656,7 @@ describe('POST /api/auth/sso/register', () => { token_endpoint_auth_methods_supported: ['client_secret_post'], }), }) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') @@ -680,7 +666,7 @@ describe('POST /api/auth/sso/register', () => { it('registers successfully when discovery is unreachable and all endpoints are explicit', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) mockSecureFetchWithPinnedIP.mockRejectedValue(new Error('ECONNREFUSED')) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.skipDiscovery).toBe(true) @@ -696,7 +682,7 @@ describe('POST /api/auth/sso/register', () => { token_endpoint_auth_methods_supported: ['client_secret_basic', 'client_secret_post'], }), }) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') @@ -708,7 +694,7 @@ describe('POST /api/auth/sso/register', () => { ok: true, json: async () => ({}), }) - const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + const res = await POST(request(OIDC_BODY)) expect(res.status).toBe(200) const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') @@ -728,7 +714,7 @@ describe('POST /api/auth/sso/register', () => { tokenEndpoint: undefined, jwksEndpoint: undefined, } - const res = await POST(request({ ...discoveredBody, orgId: 'org1' })) + const res = await POST(request(discoveredBody)) const json = await res.json() expect(res.status).toBe(400) expect(json.error).toContain('resolves to a private IP address') diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index c4991dc4271..ff657118256 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -2,13 +2,13 @@ import { db, member, ssoDomain, ssoProvider } from '@sim/db' import { createLogger } from '@sim/logger' import { getErrorMessage, getPostgresConstraintName } from '@sim/utils/errors' import { normalizeSSODomain } from '@sim/utils/sso-domain' -import { and, eq, isNull, sql } from 'drizzle-orm' +import { and, eq, sql } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { ssoRegistrationContract } from '@/lib/api/contracts/auth' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { auth, getSession } from '@/lib/auth' import { hasSSOAccess } from '@/lib/billing' -import { isHosted, isSsoEnabled } from '@/lib/core/config/env-flags' +import { isSsoEnabled } from '@/lib/core/config/env-flags' import { secureFetchWithPinnedIP, validateUrlWithDNS, @@ -116,18 +116,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const body = parsed.data.body const { providerId, issuer, providerType, mapping, orgId, jitProvisioningEnabled } = body - if (orgId) { - const [membership] = await db - .select({ organizationId: member.organizationId, role: member.role }) - .from(member) - .where(and(eq(member.userId, session.user.id), eq(member.organizationId, orgId))) - .limit(1) - if (!membership) { - return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) - } - if (membership.role !== 'owner' && membership.role !== 'admin') { - return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) - } + /** + * Always org-scoped: an org-less provider has no `sso_domain` proof, so only + * operators create one, via `packages/db/scripts/register-sso-provider.ts`. + */ + const [membership] = await db + .select({ organizationId: member.organizationId, role: member.role }) + .from(member) + .where(and(eq(member.userId, session.user.id), eq(member.organizationId, orgId))) + .limit(1) + if (!membership) { + return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) + } + if (membership.role !== 'owner' && membership.role !== 'admin') { + return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) } const domain = normalizeSSODomain(body.domain) @@ -142,20 +144,19 @@ export const POST = withRouteHandler(async (request: NextRequest) => { /** * Configuring org SSO for a domain requires DNS-proven ownership; without it * a first-come claim lets any org wire another company's domain to their own - * IdP. Migration 0266 grandfathered existing domains. Org-less SSO is not gated. + * IdP. Migration 0266 grandfathered existing domains. */ + const verifiedDomainClause = and( + eq(ssoDomain.organizationId, orgId), + eq(ssoDomain.domain, domain), + eq(ssoDomain.status, 'verified') + ) + const isOrgDomainVerified = async (): Promise => { - if (!orgId) return true const [verified] = await db .select({ id: ssoDomain.id }) .from(ssoDomain) - .where( - and( - eq(ssoDomain.organizationId, orgId), - eq(ssoDomain.domain, domain), - eq(ssoDomain.status, 'verified') - ) - ) + .where(verifiedDomainClause) .limit(1) return Boolean(verified) } @@ -173,13 +174,21 @@ export const POST = withRouteHandler(async (request: NextRequest) => { // window where the proof is removed while discovery is in flight. if (!(await isOrgDomainVerified())) return domainNotVerifiedResponse() + /** + * An org-less provider the caller created counts as theirs, so its claim on + * a domain is not reported as another tenant's. + */ const isOwnedByCaller = (provider: { userId: string | null organizationId: string | null - }): boolean => { - if (provider.userId === session.user.id && !provider.organizationId) return true - return orgId ? provider.organizationId === orgId : false - } + }): boolean => + provider.organizationId === orgId || + (provider.userId === session.user.id && !provider.organizationId) + + const ownerClause = and( + eq(ssoProvider.providerId, providerId), + eq(ssoProvider.organizationId, orgId) + ) /** * Refuses the domain when another tenant has claimed it, or when the caller @@ -236,10 +245,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const findProviderIdConflict = async () => ( await db - .select({ - userId: ssoProvider.userId, - organizationId: ssoProvider.organizationId, - }) + .select({ userId: ssoProvider.userId, organizationId: ssoProvider.organizationId }) .from(ssoProvider) .where(eq(ssoProvider.providerId, providerId)) ).find((provider) => !isOwnedByCaller(provider)) @@ -274,7 +280,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { providerId, issuer, domain, - ...(orgId ? { organizationId: orgId } : {}), + organizationId: orgId, } if (providerType === 'oidc') { @@ -292,13 +298,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => { let clientSecret = rawClientSecret if (rawClientSecret === REDACTED_MARKER) { - const ownerClause = orgId - ? and(eq(ssoProvider.providerId, providerId), eq(ssoProvider.organizationId, orgId)) - : and( - eq(ssoProvider.providerId, providerId), - eq(ssoProvider.userId, session.user.id), - isNull(ssoProvider.organizationId) - ) const [existing] = await db .select({ oidcConfig: ssoProvider.oidcConfig }) .from(ssoProvider) @@ -632,18 +631,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => { // edit through updateSSOProvider so re-saving an SSO config works instead of // failing. The verification gate above already ran against the target domain, // so an edit that moves SSO to an unverified domain is still blocked. - // The personal branch MUST require a null org: org providers store - // userId = their creator, so without it an org admin could send a - // personal-mode request (which skips the membership check and the - // verification gate) yet still match — and then update — their org's - // provider, moving it to an unverified domain. Mirrors isOwnedByCaller. - const ownerClause = orgId - ? and(eq(ssoProvider.providerId, providerId), eq(ssoProvider.organizationId, orgId)) - : and( - eq(ssoProvider.providerId, providerId), - eq(ssoProvider.userId, session.user.id), - isNull(ssoProvider.organizationId) - ) // Config columns are captured, not just the id: an update whose trust grant is // refused has to be undone, or the rejected config stays stored and goes live // the moment the domain is verified again. @@ -668,29 +655,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => { * UPDATE waits can still grant trust after ownership is gone. `FOR SHARE` * orders the two — the delete blocks until this commits, and if it committed * first the SELECT finds nothing. - * - * Org-less SSO is self-host-only (Sim's UI always registers org-scoped) and - * has no proof behind it, so it is trusted only when self-hosted. */ - const grantProviderDomainTrust = async (): Promise => { - if (!orgId) { - await db - .update(ssoProvider) - .set({ domainVerified: !isHosted, jitProvisioningEnabled }) - .where(ownerClause) - return true - } - return db.transaction(async (tx) => { + const grantProviderDomainTrust = (): Promise => + db.transaction(async (tx) => { const [proof] = await tx .select({ id: ssoDomain.id }) .from(ssoDomain) - .where( - and( - eq(ssoDomain.organizationId, orgId), - eq(ssoDomain.domain, domain), - eq(ssoDomain.status, 'verified') - ) - ) + .where(verifiedDomainClause) .limit(1) .for('share') if (!proof) return false @@ -702,7 +673,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => { .returning({ id: ssoProvider.id }) return granted.length > 0 }) - } if (existingOwnedProvider) { const revertProviderUpdate = async (): Promise => { @@ -784,12 +754,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => { // registerSSOProvider spreads the created row's `id` at runtime, but the // typed return omits it — read it defensively and only delete when it's a // real id, so a future shape change can't turn the rollback into a silent - // no-op that leaves a provider on an unverified domain. `orgId` is checked - // only to narrow it: the org-less path grants unconditionally, so a refused - // grant always means an org-scoped registration. + // no-op that leaves a provider on an unverified domain. // double-cast-allowed: Better Auth's return type omits the runtime `id` const createdRowId = (registration as unknown as { id?: unknown }).id - if (orgId && typeof createdRowId === 'string' && createdRowId.length > 0) { + if (typeof createdRowId === 'string' && createdRowId.length > 0) { await db .delete(ssoProvider) .where(and(eq(ssoProvider.id, createdRowId), eq(ssoProvider.organizationId, orgId))) diff --git a/apps/sim/app/api/v2/knowledge/search/route.provenance.test.ts b/apps/sim/app/api/v2/knowledge/search/route.provenance.test.ts index 3971a4c4ca5..7e847cb2797 100644 --- a/apps/sim/app/api/v2/knowledge/search/route.provenance.test.ts +++ b/apps/sim/app/api/v2/knowledge/search/route.provenance.test.ts @@ -59,7 +59,10 @@ vi.mock('@/lib/knowledge/embeddings', () => ({ vi.mock('@/lib/knowledge/search/queries', () => ({ generateSearchEmbedding: mocks.generateEmbedding, - executeKnowledgeSearch: mocks.executeSearch, + retrieveKnowledgeSearch: async (...args: unknown[]) => ({ + rows: await mocks.executeSearch(...args), + retrieval: { status: 'complete', timedOutLegs: [] }, + }), getDocumentMetadataByIds: mocks.getDocumentMetadata, })) diff --git a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.test.tsx b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.test.tsx index 8a39e94a9fe..0d1077648d7 100644 --- a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.test.tsx +++ b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.test.tsx @@ -5,19 +5,27 @@ import { act, type ComponentProps } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockNavigate, mockPush } = vi.hoisted(() => ({ +const { mockNavigate, mockPush, context } = vi.hoisted(() => ({ mockNavigate: vi.fn(), mockPush: vi.fn(), + context: { + organization: { id: 'org-1' }, + }, })) vi.mock('next/navigation', () => ({ useRouter: () => ({ push: mockPush }), + usePathname: () => '/o/org-1/home', })) vi.mock('next/link', () => ({ default: ({ onNavigate, + prefetch: _prefetch, ...props - }: ComponentProps<'a'> & { onNavigate?: (event: { preventDefault: () => void }) => void }) => ( + }: ComponentProps<'a'> & { + prefetch?: boolean + onNavigate?: (event: { preventDefault: () => void }) => void + }) => ( ({ /> ), })) +vi.mock('@/lib/auth/sign-out', () => ({ signOutAndRedirect: vi.fn() })) vi.mock('@/lib/desktop', () => ({ getDesktopUpdates: () => null })) vi.mock('@/hooks/use-desktop-update-state', () => ({ useDesktopUpdateState: () => ({ status: 'idle' }), @@ -42,12 +51,14 @@ vi.mock('@/hooks/queries/user-profile', () => ({ useUserProfile: () => ({ data: { id: 'user-1', name: 'Ada', email: 'ada@example.com' } }), })) vi.mock('@/app/o/[organizationId]/providers/organization-provider', () => ({ - useOrganizationContext: () => ({ organization: { id: 'org-1' } }), + useOrganizationContext: () => context, })) vi.mock('@/app/workspace/[workspaceId]/w/components/sidebar/components', () => ({ SidebarTooltip: ({ children }: { children: React.ReactNode }) => children, })) -vi.mock('@/components/icons', () => ({ SlackIcon: () => })) +vi.mock('@/components/icons', () => ({ + SlackIcon: () => , +})) import { OrganizationFooter } from '@/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer' import { useSettingsDirtyStore } from '@/stores/settings/dirty/store' @@ -71,7 +82,7 @@ afterEach(async () => { vi.unstubAllGlobals() }) -async function selectSettings() { +async function openProfileMenu() { await act(async () => { root.render( {}} onJoinSlack={() => {}} + onContactSupport={() => {}} /> ) }) @@ -88,15 +100,27 @@ async function selectSettings() { await act(async () => { trigger.dispatchEvent(new MouseEvent('pointerdown', { bubbles: true, button: 0 })) }) +} + +async function selectSettings() { + await openProfileMenu() const link = document.querySelector('a[href="/o/org-1/settings/general"]') if (!link) throw new Error('Settings link is missing') await act(async () => link.click()) } describe('OrganizationFooter settings navigation', () => { + it('keeps only Settings and Sign out in the organization profile menu', async () => { + await openProfileMenu() + expect( + [...document.querySelectorAll('[role="menuitem"]')].map((item) => item.textContent) + ).toEqual(['Settings', 'Sign out']) + expect(document.querySelector('[role="separator"]')).toBeNull() + }) + it('navigates immediately when settings are clean', async () => { await selectSettings() - expect(mockNavigate).toHaveBeenCalledWith('/o/org-1/settings/general') + expect(mockPush).toHaveBeenCalledWith('/o/org-1/settings/general') expect(useSettingsDirtyStore.getState().pendingLeave).toBeNull() }) diff --git a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.tsx b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.tsx index ecc37a12e01..125ab4202af 100644 --- a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.tsx +++ b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.tsx @@ -1,242 +1,28 @@ 'use client' -import type { DesktopUpdateState } from '@sim/desktop-bridge' -import { - Chip, - chipContentLabelClass, - chipPrimaryFillTokens, - chipVariants, - cn, - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuItemLabel, - DropdownMenuSeparator, - DropdownMenuTrigger, - OverflowText, - Skeleton, -} from '@sim/emcn' -import { BookOpen, Download, HelpCircle, Settings } from '@sim/emcn/icons' -import { SlackIcon } from '@/components/icons' -import { SettingsGuardedLink } from '@/components/settings/settings-guarded-link' -import { getDesktopUpdates } from '@/lib/desktop' +import type { ComponentProps } from 'react' +import { useRouter } from 'next/navigation' import { organizationRoutes } from '@/lib/navigation/paths' -import { getUserColor } from '@/lib/workspaces/colors' import { useOrganizationContext } from '@/app/o/[organizationId]/providers/organization-provider' -import { SidebarTooltip } from '@/app/workspace/[workspaceId]/w/components/sidebar/components' -import { - SIDEBAR_ITEM_GAP_CLASS, - SIDEBAR_RAIL_CHIP_CLASS, -} from '@/app/workspace/[workspaceId]/w/components/sidebar/constants' -import { useUserProfile } from '@/hooks/queries/user-profile' -import { useDesktopUpdateState } from '@/hooks/use-desktop-update-state' +import { SidebarFooter } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer' -function hasAvailableDesktopUpdate(state: DesktopUpdateState): boolean { - return state.status === 'available' || state.status === 'downloading' || state.status === 'ready' -} - -function desktopUpdateActionLabel(state: DesktopUpdateState): string { - if (state.status === 'downloading') { - return state.percent === undefined - ? 'Downloading update…' - : `Downloading update ${state.percent}%` - } - return state.status === 'ready' ? 'Restart to update' : 'Update' -} - -/** Compact primary update circle using the same footprint as the surrounding sidebar icons. */ -function DesktopUpdateIcon({ className }: { className?: string }) { - return ( -
- {/* Download's default viewBox is asymmetric around its paths. Center the - artwork itself, not merely its SVG box, inside the avatar-sized circle. */} - -
- ) -} +interface OrganizationFooterProps + extends Omit< + ComponentProps, + 'accountSettingsHref' | 'onOpenAccountSettings' | 'navigationLinks' + > {} -interface OrganizationFooterProps { - /** - * True while the scroll region above still hides rows beyond its bottom edge — - * the same test the divider under the pinned nav applies at the top. The bar's - * top rule is drawn only then, so a list that fits meets the footer with no line. - */ - showDivider: boolean - isCollapsed: boolean - showCollapsedTooltips: boolean - onOpenDocs: () => void - onJoinSlack: () => void -} - -/** - * Pinned bottom bar of the organization sidebar: the viewer's avatar and name, - * which open their account settings, plus a help menu. Same two elements and the - * same layout as the workspace footer — expanded they share one row with help hard - * right, collapsed they stack as icon chips with help on top. - * - * Collapsed reverses the flex direction instead of reordering the DOM, which keeps - * both elements (and the help menu's trigger) alive across a toggle. - */ -export function OrganizationFooter({ - showDivider, - isCollapsed, - showCollapsedTooltips, - onOpenDocs, - onJoinSlack, -}: OrganizationFooterProps) { +export function OrganizationFooter(props: OrganizationFooterProps) { const { organization } = useOrganizationContext() - const { data: profile } = useUserProfile() - const updateState = useDesktopUpdateState() - - const name = profile ? profile.name?.trim() || profile.email : '' - const updateAvailable = hasAvailableDesktopUpdate(updateState) - - const handleUpdateSelect = () => { - const updates = getDesktopUpdates() - if (updateState.status === 'ready') { - updates?.install() - } else if (updateState.status === 'available') { - updates?.check() - } - } - - /** - * Plain `img`/`div` rather than the emcn `Avatar`, whose Radix root renders a - * `` — and globals fade every `span` in the collapsed rail to `opacity: 0`, - * which would blank the avatar exactly where it is the only thing left to see. - */ - const avatar = !profile ? ( - - ) : profile.image ? ( - - ) : ( -
- {name.charAt(0).toUpperCase()} -
- ) - - /** - * Expanded, the chip hugs its content (`max-w-full` so a long name truncates - * rather than overflowing); collapsed, `fullWidth` fills the narrow rail and - * `min-w-0` lets the hidden label give up its box so the chip never overflows it. - * The name is the button's accessible name — no `aria-label`, which would - * override the visible text. - */ - const profileMenu = ( - - - - - - - - - - - - - - - - ) - - /** - * One node across both states; only `fullWidth` changes, so the same Radix menu - * survives the transition. `shrink-0` keeps the chip off the avatar while the rail - * is briefly narrower than the row — the aside's clip hides it until there is room. - */ - const helpMenu = ( - - - - - - - {/* Anchored to whichever edge the trigger sits on, so the menu never overhangs the rail. */} - - {updateAvailable && ( - <> - - - {desktopUpdateActionLabel(updateState)} - - - - )} - - - Docs - - - - Join Slack - - - - ) + const router = useRouter() + const accountSettingsHref = organizationRoutes(organization.id).settingsSection('general') return ( -
- {/* Expanded, claims the row's free width so the help button lands hard right. - `flex` makes the inline-flex chip a flex item, so the wrapper is exactly the - chip's 30px rather than a line box padded by the strut's half-leading. */} -
{profileMenu}
- {helpMenu} -
+ router.push(accountSettingsHref)} + navigationLinks={[]} + /> ) } diff --git a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.test.tsx b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.test.tsx index 665eea639dc..e3d69feee22 100644 --- a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.test.tsx +++ b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.test.tsx @@ -7,7 +7,13 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const mocks = vi.hoisted(() => ({ upload: vi.fn(), refresh: vi.fn() })) +const mocks = vi.hoisted(() => ({ upload: vi.fn(), refresh: vi.fn(), invite: vi.fn() })) +vi.mock('@/app/workspace/[workspaceId]/components/invite-modal', () => ({ + InviteModal: (props: object) => { + mocks.invite(props) + return null + }, +})) vi.mock('@/lib/uploads/client/session-upload', () => ({ uploadInternalFileSession: mocks.upload, })) @@ -49,7 +55,12 @@ afterEach(async () => { vi.unstubAllGlobals() }) -async function render(canEditLogo = true, isCollapsed = false, onExpandSidebar = vi.fn()) { +async function render( + canEditLogo = true, + isCollapsed = false, + onExpandSidebar = vi.fn(), + canInviteMembers = canEditLogo +) { await act(async () => { root.render( @@ -57,6 +68,7 @@ async function render(canEditLogo = true, isCollapsed = false, onExpandSidebar = @@ -189,3 +201,35 @@ describe('OrganizationHeader logo upload', () => { expect(mocks.refresh).toHaveBeenCalledOnce() }) }) + +describe('OrganizationHeader member actions', () => { + it('opens the existing invitation flow for the current organization', async () => { + await render() + await openMenu() + const invite = [...document.querySelectorAll('[role="menuitem"]')].find( + (item) => item.textContent === 'Invite people' + )! + await act(async () => invite.click()) + expect(mocks.invite).toHaveBeenCalledWith( + expect.objectContaining({ + open: true, + organizationId: 'org-1', + isOrganizationAdmin: true, + canInvite: true, + }) + ) + }) + + it.each([false, true])( + 'keeps settings access but hides disallowed invitations (admin=%s)', + async (admin) => { + await render(admin, false, vi.fn(), false) + await openMenu() + expect(document.querySelector('a[href="/o/org-1/settings/members"]')).toHaveTextContent( + 'Settings' + ) + expect(document.querySelector('[role="menu"]')).not.toHaveTextContent('Invite people') + expect(mocks.invite).not.toHaveBeenCalled() + } + ) +}) diff --git a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx index 19a580ae22b..3075628dbc7 100644 --- a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx +++ b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx @@ -1,6 +1,6 @@ 'use client' -import { useRef } from 'react' +import { useRef, useState } from 'react' import { Chip, ChipChevronDown, @@ -12,13 +12,14 @@ import { Tooltip, toast, } from '@sim/emcn' -import { PanelLeft, Settings } from '@sim/emcn/icons' +import { PanelLeft, Send, Settings } from '@sim/emcn/icons' import { useRouter } from 'next/navigation' import { IdentityTile } from '@/components/identity-tile/identity-tile' import { getOrganizationSettingsHref } from '@/components/settings/navigation' import { SettingsGuardedLink } from '@/components/settings/settings-guarded-link' import type { OrganizationSurfaceOrganization } from '@/lib/organizations/surface' import { LOGO_ACCEPT_ATTRIBUTE } from '@/lib/uploads/client/logo-file' +import { InviteModal } from '@/app/workspace/[workspaceId]/components/invite-modal' import { SIDEBAR_RAIL_CHIP_CLASS } from '@/app/workspace/[workspaceId]/w/components/sidebar/constants' import { useUploadOrganizationLogo } from '@/hooks/queries/organization-logo' @@ -29,6 +30,7 @@ function getOrganizationInitial(name: string): string { interface OrganizationHeaderProps { organization: OrganizationSurfaceOrganization canEditLogo: boolean + canInviteMembers: boolean isCollapsed: boolean /** Expands the rail; the collapsed header is itself the expand control. */ onExpandSidebar: () => void @@ -44,6 +46,7 @@ interface OrganizationHeaderProps { export function OrganizationHeader({ organization, canEditLogo, + canInviteMembers, isCollapsed, onExpandSidebar, }: OrganizationHeaderProps) { @@ -52,6 +55,7 @@ export function OrganizationHeader({ const { mutate: uploadLogo, isPending: isUploadingLogo } = useUploadOrganizationLogo( organization.id ) + const [isInviteModalOpen, setIsInviteModalOpen] = useState(false) const initial = getOrganizationInitial(organization.name) if (isCollapsed) { @@ -131,7 +135,7 @@ export function OrganizationHeader({ aria-label='Change organization logo' aria-busy={isUploadingLogo} textValue='Change organization logo' - className='h-auto shrink-0 p-1' + className='h-auto shrink-0 p-0 hover-hover:opacity-70 focus-visible:opacity-70' disabled={isUploadingLogo} onSelect={(event) => { event.preventDefault() @@ -161,8 +165,23 @@ export function OrganizationHeader({ Settings + {canInviteMembers && ( + setIsInviteModalOpen(true)}> + + Invite people + + )} + {isInviteModalOpen && ( + + )} ) } diff --git a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list.test.tsx b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list.test.tsx index e4ce1404ee2..2a08c775fb9 100644 --- a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list.test.tsx +++ b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list.test.tsx @@ -45,6 +45,22 @@ vi.mock('@/hooks/queries/workspace', () => ({ })) import { WorkspaceList } from '@/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list' +import { useHoverMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-hover-menu' + +function KeyboardWorkspaceFlyout() { + const hover = useHoverMenu() + return ( + (open ? hover.open() : hover.close())} + > + Workspaces + + + + + ) +} let container: HTMLDivElement let root: Root @@ -91,6 +107,18 @@ async function render() { } describe('WorkspaceList rail view', () => { + it('keeps the flyout open when opened from the keyboard without pointer hover', async () => { + workspacesState.workspaces = [{ id: 'ws-1', name: 'Design' }] + await act(async () => root.render()) + const trigger = container.querySelector('button')! + await act(async () => { + trigger.focus() + trigger.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })) + }) + expect(document.querySelector('[role="menu"]')).not.toBeNull() + expect(document.querySelector('a[href="/workspace/ws-1"]')).toHaveTextContent('Design') + }) + it('lists every workspace as a link into it', async () => { workspacesState.workspaces = [ { id: 'ws-1', name: 'Design' }, diff --git a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list.tsx b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list.tsx index c0085b6b92c..8d1bfa0b942 100644 --- a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list.tsx +++ b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/workspaces-section/workspace-list.tsx @@ -52,11 +52,13 @@ export function WorkspaceList({ organizationId, pathname, flyout }: WorkspaceLis }, }) const lockFlyout = flyout?.setLocked + const isInteracting = menu.isOpen || rename.editingId !== null useEffect(() => { - lockFlyout?.(menu.isOpen || rename.editingId !== null) - return () => lockFlyout?.(false) - }, [lockFlyout, menu.isOpen, rename.editingId]) + if (!lockFlyout || !isInteracting) return + lockFlyout(true) + return () => lockFlyout(false) + }, [lockFlyout, isInteracting]) const visibleWorkspaces = flyout ? workspaces : workspaces.slice(0, visibleCount) const hasMore = workspaces.length > visibleCount @@ -90,7 +92,11 @@ export function WorkspaceList({ organizationId, pathname, flyout }: WorkspaceLis initial={getWorkspaceInitial(workspace.name)} logoUrl={workspace.logoUrl} /> - + ) const onMoreClick = (event: React.MouseEvent) => { @@ -136,6 +142,16 @@ export function WorkspaceList({ organizationId, pathname, flyout }: WorkspaceLis onPointerMove={(event) => { if (menu.isOpen || rename.editingId) event.preventDefault() }} + actionIndicator={ + isPinned ? ( + + ) : undefined + } action={ openMenu(event, workspace.id)} > {label} - {isPinned && } ) diff --git a/apps/sim/app/o/[organizationId]/components/organization-sidebar/organization-sidebar.tsx b/apps/sim/app/o/[organizationId]/components/organization-sidebar/organization-sidebar.tsx index 363e229f6ec..c4434335f55 100644 --- a/apps/sim/app/o/[organizationId]/components/organization-sidebar/organization-sidebar.tsx +++ b/apps/sim/app/o/[organizationId]/components/organization-sidebar/organization-sidebar.tsx @@ -27,6 +27,7 @@ import { useSidebarChrome } from '@/app/workspace/[workspaceId]/components/works import { useRegisterGlobalCommands } from '@/app/workspace/[workspaceId]/providers/global-commands-provider' import { createCommands } from '@/app/workspace/[workspaceId]/utils/commands-utils' import { + HelpModal, isNavItemActive, NavItemContextMenu, SidebarNavChip, @@ -94,6 +95,7 @@ export const OrganizationSidebar = memo(function OrganizationSidebar() { const settingsPath = organizationRoutes(organization.id).settings const isSettings = pathname === settingsPath || pathname?.startsWith(`${settingsPath}/`) + const [isHelpModalOpen, setIsHelpModalOpen] = useState(false) const [menuHref, setMenuHref] = useState(null) const { isOpen: isHrefMenuOpen, @@ -182,6 +184,7 @@ export const OrganizationSidebar = memo(function OrganizationSidebar() { @@ -282,6 +285,7 @@ export const OrganizationSidebar = memo(function OrganizationSidebar() { showCollapsedTooltips={showCollapsedTooltips} onOpenDocs={handleOpenDocs} onJoinSlack={handleOpenSlackCommunity} + onContactSupport={() => setIsHelpModalOpen(true)} /> + + {/* Not on the peek card: the resize hook writes an inline `--sidebar-width` that out-specifies the `[data-peek]` rule, stranding the card at a stale width. */} {!isPeeking && ( diff --git a/apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/page.tsx b/apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/page.tsx index ebabd37f6fc..6cd806b52bc 100644 --- a/apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/page.tsx +++ b/apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/page.tsx @@ -1,16 +1,21 @@ import { ChipLink } from '@sim/emcn' import { notFound, redirect } from 'next/navigation' +import type { SearchParams } from 'nuqs/server' import { readSearchDocumentResultSchema } from '@/lib/api/contracts/knowledge/documents' import { getSession } from '@/lib/auth' import { OrchestrationError } from '@/lib/core/orchestration/types' import { readSearchDocument } from '@/lib/knowledge/application/read-search-document' import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect' +import { + loadDocumentReadParams, + serializeDocumentReadParams, +} from '@/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/search-params' import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection' import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' interface OrganizationDocumentPageProps { params: Promise<{ organizationId: string; knowledgeBaseId: string; documentId: string }> - searchParams: Promise<{ offset?: string }> + searchParams: Promise } export default async function OrganizationDocumentPage({ @@ -18,15 +23,15 @@ export default async function OrganizationDocumentPage({ searchParams, }: OrganizationDocumentPageProps) { const { organizationId, knowledgeBaseId, documentId } = await params - const { offset: rawOffset } = await searchParams - const offset = rawOffset === undefined ? 0 : Number(rawOffset) - if (!Number.isInteger(offset) || offset < 0 || offset > 5000) notFound() + const position = await loadDocumentReadParams(searchParams, { strict: true }).catch(() => + notFound() + ) const href = `/o/${encodeURIComponent(organizationId)}/knowledge/${encodeURIComponent(knowledgeBaseId)}/${encodeURIComponent(documentId)}` const session = await getSession() if (!session?.user) { redirect( buildAuthCrossLink('/login', { - callbackUrl: offset ? `${href}?offset=${offset}` : href, + callbackUrl: serializeDocumentReadParams(href, position), isInviteFlow: false, }) ) @@ -39,15 +44,15 @@ export default async function OrganizationDocumentPage({ input: { documentId, assertedOrganizationId: organizationId, - offset, - limit: 20, + ...position, + limit: 3, resultSecretRegistry: registry, }, }) } catch (error) { if ( error instanceof OrchestrationError && - (error.code === 'not_found' || error.code === 'forbidden') + (error.code === 'not_found' || error.code === 'forbidden' || error.code === 'validation') ) notFound() throw error @@ -71,11 +76,11 @@ export default async function OrganizationDocumentPage({

))} diff --git a/apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/search-params.ts b/apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/search-params.ts new file mode 100644 index 00000000000..42f9dd224e8 --- /dev/null +++ b/apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/search-params.ts @@ -0,0 +1,25 @@ +import { createLoader, createParser, createSerializer } from 'nuqs/server' + +const parseAsDocumentPosition = createParser({ + parse: (value) => { + if (!/^\d+$/.test(value)) return null + const position = Number(value) + return Number.isSafeInteger(position) && position <= 2147483647 ? position : null + }, + serialize: String, +}).withDefault(0) + +export const documentReadParams = { + startChunkIndex: parseAsDocumentPosition, + startOffset: parseAsDocumentPosition, +} + +const documentReadUrlKeys = { + urlKeys: { + startChunkIndex: 'start-chunk-index', + startOffset: 'start-offset', + }, +} as const + +export const loadDocumentReadParams = createLoader(documentReadParams, documentReadUrlKeys) +export const serializeDocumentReadParams = createSerializer(documentReadParams, documentReadUrlKeys) diff --git a/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx index b94535be58c..c6050bf0f43 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx @@ -347,6 +347,8 @@ describe('ConnectOAuthModal reauthorization', () => { }, }) + expect(container.querySelector('header')).toHaveTextContent('Reconnect Slack') + expect(container).not.toHaveTextContent('tool requires access') await clickConnect() expect(mocks.writeOAuthReturnContext).toHaveBeenCalledExactlyOnceWith( @@ -424,6 +426,8 @@ describe('ConnectOAuthModal reauthorization', () => { }, }) + expect(container.querySelector('header')).toHaveTextContent('Connect Slack') + expect(container).toHaveTextContent('tool requires access') await clickConnect() expect(mocks.writeOAuthReturnContext).toHaveBeenCalledOnce() diff --git a/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx b/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx index 4ec62347415..e624f434d0e 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx @@ -506,7 +506,16 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) { ? `An integration named "${existingCredential.displayName}" already exists.` : undefined) - const title = `Connect ${providerName}` + const isConnectorReconnect = !isConnect && props.returnContext?.origin === 'kb-connectors' + const connectLabel = isConnectorReconnect + ? newScopes.length > 0 + ? 'Update access' + : 'Reconnect' + : 'Connect' + const title = + isConnectorReconnect && newScopes.length > 0 + ? `Update ${providerName} access` + : `${connectLabel} ${providerName}` return ( @@ -519,7 +528,11 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) { {!isConnect && (

- The "{props.toolName}" tool requires access to your account. + {isConnectorReconnect + ? newScopes.length > 0 + ? 'Approve the requested permissions to continue syncing.' + : `Continue to ${providerName} to restore this connection.` + : `The "${props.toolName}" tool requires access to your account.`}

)} @@ -651,7 +664,7 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) { : undefined } primaryAction={{ - label: isPending ? 'Connecting...' : 'Connect', + label: isPending ? 'Connecting...' : connectLabel, onClick: handleConnect, disabled: isDisabled, }} diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-disclosure.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-disclosure.tsx index c25f9b774e3..724f22c1929 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-disclosure.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-disclosure.tsx @@ -1,15 +1,16 @@ 'use client' import { type ReactNode, useId } from 'react' -import { ChevronDown, cn, Expandable, ExpandableContent } from '@sim/emcn' +import { ChevronDown, cn, Expandable, ExpandableContent, handleKeyboardActivation } from '@sim/emcn' import { ActivityViewport } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-viewport' -interface ActivityDisclosureProps { +export interface ActivityDisclosureProps { header: ReactNode children: ReactNode expanded: boolean onToggle: () => void isStreaming: boolean + collapsible?: boolean unbounded?: boolean } @@ -21,39 +22,51 @@ export function ActivityDisclosure({ onToggle, isStreaming, unbounded = false, + collapsible = true, }: ActivityDisclosureProps) { const contentId = useId() const headerId = useId() return ( -
- - - - - {children} - - - + {collapsible && ( + + )} +
+ {collapsible && ( + + +
+ + {children} + +
+ + + )} ) } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx new file mode 100644 index 00000000000..9566b47c9fb --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx @@ -0,0 +1,175 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { AgentGroup } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group' +import type { AgentGroupItem } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view' +import type { ToolCallData, ToolCallStatus } from '@/app/workspace/[workspaceId]/home/types' + +function tool(id: string, status: ToolCallStatus = 'executing'): ToolCallData { + return { id, toolName: 'read', displayTitle: `Reading ${id}`, status } +} + +function items(tools: ToolCallData[]): AgentGroupItem[] { + return tools.map((data) => ({ type: 'tool', data })) +} + +describe.each(['mothership', 'workflow', 'browser'])('%s activity cadence', (agentName) => { + let root: Root + let container: HTMLDivElement + beforeEach(() => { + vi.useFakeTimers() + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + afterEach(() => { + act(() => root.unmount()) + container.remove() + vi.useRealTimers() + }) + const render = (tools: ToolCallData[], active = true) => + act(() => + root.render( + + ) + ) + const header = () => container.querySelector('[role="status"]') + const advance = (ms: number) => act(() => vi.advanceTimersByTime(ms)) + + it('shows the first action immediately and coalesces bursts without replaying a backlog', () => { + render([]) + advance(100) + render([tool('first')]) + expect(header()?.textContent).toBe('Reading first') + const row = header() + const shimmer = container.querySelector('[class*="shimmer"]') + advance(100) + render([tool('first', 'success')]) + expect(header()?.textContent).toBe('Reading first') + expect(container.querySelector('[class*="shimmer"]')).toBe(shimmer) + render([tool('first', 'success'), tool('second')]) + expect(header()).toBe(row) + expect(container.querySelector('[class*="shimmer"]')).toBe(shimmer) + expect(header()?.textContent).toBe('Reading first') + advance(600) + render([tool('first', 'success'), tool('second', 'success'), tool('third')]) + advance(299) + expect(header()?.textContent).toBe('Reading first') + advance(1) + expect(header()?.textContent).toBe('Reading third') + expect(container.textContent).not.toContain('Agent prefix') + advance(1000) + expect(header()?.textContent).toBe('Reading third') + }) + + it('keeps live history complete under a stable expanded header with keyboard disclosure', () => { + render([tool('first', 'success'), tool('second')]) + const trigger = container.querySelector('[role="button"]')! + const event = new KeyboardEvent('keydown', { key: ' ', bubbles: true, cancelable: true }) + act(() => trigger.dispatchEvent(event)) + expect(event.defaultPrevented).toBe(true) + expect(trigger.getAttribute('aria-expanded')).toBe('true') + expect(header()?.textContent).toBe('Tool activity') + render([tool('first', 'success'), tool('second', 'success'), tool('third')]) + expect(header()?.textContent).toBe('Tool activity') + expect(container.querySelector('[data-state="open"]')?.textContent).toBe( + 'Read firstRead secondReading third' + ) + act(() => trigger.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))) + expect(trigger.getAttribute('aria-expanded')).toBe('false') + expect(header()?.textContent).toBe('Reading third') + }) + + it.each(['error', 'cancelled', 'interrupted', 'rejected', 'skipped'] as const)( + 'shows %s immediately and cancels a pending cosmetic update', + (status) => { + render([tool('first')]) + advance(100) + render([tool('first', 'success'), tool('second')]) + render([tool('first', 'success'), tool('second', status)]) + const prefix = + status === 'error' || status === 'rejected' + ? 'Failed' + : status === 'skipped' + ? 'Skipped' + : 'Stopped' + expect(header()?.textContent).toBe(`${prefix} reading second`) + expect(container.querySelector('[class*="shimmer"]')).toBeNull() + advance(1500) + expect(header()?.textContent).toBe(`${prefix} reading second`) + } + ) + + it('shows final completion immediately and never replays the held action', () => { + render([tool('first')]) + advance(100) + render([tool('first', 'success'), tool('second')]) + render([tool('first', 'success'), tool('second', 'success')], false) + expect(header()?.textContent).toBe('Read files') + expect(container.querySelector('[class*="shimmer"]')).toBeNull() + advance(2000) + expect(header()?.textContent).toBe('Read files') + }) + + it.each(['error', 'cancelled', 'interrupted', 'rejected', 'skipped'] as const)( + 'keeps an earlier parallel call active when the latest one becomes %s', + (status) => { + render([tool('first'), tool('second')]) + advance(100) + render([tool('first'), tool('second', status)]) + const outcome = + status === 'error' || status === 'rejected' + ? 'failed' + : status === 'skipped' + ? 'skipped' + : 'stopped' + expect(header()?.textContent).toBe(`Reading first · 1 ${outcome}`) + expect(container.querySelector('[class*="shimmer"]')).not.toBeNull() + advance(1000) + expect(header()?.textContent).toBe(`Reading first · 1 ${outcome}`) + } + ) + + it('surfaces an earlier parallel failure while the latest call keeps working', () => { + render([tool('first'), tool('second')]) + advance(100) + render([tool('first', 'error'), tool('second')]) + expect(header()?.textContent).toBe('Reading second · 1 failed') + }) + + it('keeps narration from prematurely completing an open lane', () => { + act(() => + root.render( + + ) + ) + const rows = container.querySelectorAll('[role="status"]') + if (agentName === 'mothership') { + expect(rows[0].textContent).toBe('Read first') + expect(rows[0].querySelector('[class*="shimmer"]')).toBeNull() + } + const liveRow = rows[rows.length - 1] + expect(liveRow.textContent).toBe('Reading second') + expect(liveRow.querySelector('[class*="shimmer"]')).not.toBeNull() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.tsx new file mode 100644 index 00000000000..df8539e20c2 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.tsx @@ -0,0 +1,79 @@ +'use client' + +import { useEffect, useState } from 'react' +import { ActivityStatus, type ActivityStatusProps } from '@/components/ui/activity-status' +import { + ActivityDisclosure, + type ActivityDisclosureProps, +} from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-disclosure' + +const ACTIVITY_UPDATE_INTERVAL_MS = 1000 + +interface ActivityStreamProps extends Omit { + activity: ActivityStatusProps + activityKey?: string + attentionKey: string + collapsible: boolean +} + +/** Pace only the header; history and interactive tool state continue updating immediately. */ +export function ActivityStream({ + activity, + activityKey, + attentionKey, + expanded, + collapsible, + children, + onToggle, + isStreaming, + unbounded, +}: ActivityStreamProps) { + const isExpanded = collapsible && expanded + const key = `${activityKey}:${activity.label}` + const resetKey = `${activity.isActive}:${isExpanded}:${Boolean(activityKey)}:${attentionKey}` + const [visible, setVisible] = useState(() => ({ + activity, + key, + resetKey, + shownAt: Date.now(), + })) + + /** Completion, attention, and disclosure changes bypass the cosmetic delay. */ + if (visible.resetKey !== resetKey) { + setVisible({ activity, key, resetKey, shownAt: Date.now() }) + } + + useEffect(() => { + if (!activity.isActive || isExpanded || key === visible.key) return + const remaining = Math.max(0, ACTIVITY_UPDATE_INTERVAL_MS - (Date.now() - visible.shownAt)) + const flush = () => setVisible({ activity, key, resetKey, shownAt: Date.now() }) + if (remaining === 0) { + flush() + return + } + const timer = setTimeout(flush, remaining) + return () => clearTimeout(timer) + }, [activity, key, resetKey, isExpanded, visible.key, visible.shownAt]) + + const displayed = + !activity.isActive || isExpanded || key === visible.key ? activity : visible.activity + const header = ( + + ) + return ( + + {children} + + ) +} diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view.tsx index c9733459807..fc41c6ae98b 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view.tsx @@ -1,16 +1,23 @@ 'use client' -import { type ComponentType, type ReactNode, useMemo, useState } from 'react' -import { ActivityStatus } from '@/components/ui/activity-status' +import { type ComponentType, type ReactNode, useState } from 'react' import { isBrowserAgentAvailable } from '@/lib/browser-agent/transport' import { RETIRED_BROWSER_REQUEST_TAKEOVER_ID } from '@/lib/copilot/tools/retired-tools' -import { ActivityDisclosure } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-disclosure' +import { getToolStatusDisplayTitle } from '@/lib/copilot/tools/tool-display' +import { ActivityStream } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream' import { BrowserAgentIcon } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/browser-agent-icon' import { renderInlineMarkdown } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/inline-markdown' import { MainAgentActivity } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/main-agent-activity' -import { getToolActivitySummary } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group' +import { + getActiveToolActivityTitle, + getActivityStatusTool, + getToolActivitySummary, +} from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group' import type { ToolCallItemProps } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item' -import { needsToolInput } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-interactions' +import { + getActivityAttentionKey, + needsToolInput, +} from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-interactions' import { getAgentIcon, isToolDone, @@ -43,7 +50,7 @@ export interface AgentGroupProps { items: AgentGroupItem[] isDelegating?: boolean isStreaming?: boolean - /** The subagent lane is still open (no subagent_end yet) — i.e. actively running. */ + /** This lane can receive work; main lanes close when a later transcript segment begins. */ isLaneOpen?: boolean /** Opens a subagent group on first render. */ defaultExpanded?: boolean @@ -51,8 +58,13 @@ export interface AgentGroupProps { autoScrollActivity?: boolean } -function toolStatusTitle(tool: ToolCallData): string { - return tool.displayTitle || String(tool.toolName ?? '') +function activeToolTitle(tool: ToolCallData): string { + return getToolStatusDisplayTitle( + tool.displayTitle || String(tool.toolName ?? ''), + tool.status === ToolCallStatus.success ? ToolCallStatus.executing : tool.status, + tool.toolName, + tool.activityDescription + ) } /** @@ -140,7 +152,6 @@ interface AgentGroupViewProps extends AgentGroupProps { export function AgentGroupView({ agentName, - agentLabel, items, isDelegating = false, isStreaming = false, @@ -158,28 +169,8 @@ export function AgentGroupView({ ) const isMainAgent = agentName === 'mothership' - /** Open lanes surface their latest work, including work delegated to nested agents. */ - const status = useMemo(() => { - if (isMainAgent || !isLaneOpen) return undefined - const tools = collectGroupTools(items) - const running = tools.filter((tool) => tool.status === ToolCallStatus.executing) - if (running.length > 0) { - const latest = running.reduce((newest, tool) => - (tool.startedAt ?? 0) >= (newest.startedAt ?? 0) ? tool : newest - ) - const title = toolStatusTitle(latest) - return running.length > 1 ? `${title} + ${running.length - 1}` : title - } - const last = tools.at(-1) - return last ? toolStatusTitle(last) : undefined - }, [isLaneOpen, isMainAgent, items]) - const completedTools = !isMainAgent && !isLaneOpen ? collectGroupTools(items) : [] - const headerText = status - ? `${agentLabel} — ${status}` - : completedTools.length > 0 - ? `${agentLabel} — ${getToolActivitySummary(completedTools)}` - : agentLabel - const hasItems = items.length > 0 + const tools = isMainAgent ? [] : collectGroupTools(items) + const statusTool = getActivityStatusTool(tools) const resolved = isAgentGroupResolved(items) const browserAgentAvailable = isBrowserAgentAvailable() const activeBrowserTakeover = @@ -213,6 +204,7 @@ export function AgentGroupView({ toolCallId={item.data.id} toolName={item.data.toolName} displayTitle={item.data.displayTitle} + activityDescription={item.data.activityDescription} status={item.data.status} params={item.data.params} result={item.data.result} @@ -252,28 +244,49 @@ export function AgentGroupView({ ToolCallComponent={ToolCallComponent} renderItem={renderItem} autoScrollActivity={autoScrollActivity} + isActive={isStreaming && isLaneOpen} /> ) : (
{items.map(renderItem)}
) - const header = + const headerText = isWorking + ? statusTool + ? getActiveToolActivityTitle(activeToolTitle(statusTool), statusTool, tools) + : 'Thinking' + : tools.length > 0 + ? getToolActivitySummary(tools) + : 'Tool activity' + const headerActive = + isWorking && + (!statusTool || + statusTool.status === ToolCallStatus.executing || + statusTool.status === ToolCallStatus.success) + const collapsible = + items.length > 1 || + items.some( + (item) => + item.type !== 'tool' || + needsToolInput(item.data) || + item.data.toolName === RETIRED_BROWSER_REQUEST_TAKEOVER_ID + ) return (
{isMainAgent ? ( activity - ) : hasItems ? ( - {activity} - - ) : ( - header + )} {activeBrowserTakeover && (
diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts index 8dd4c725517..1b1075a60ce 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts @@ -118,8 +118,47 @@ describe('AgentGroup inline main activity', () => { afterEach(() => { act(() => root.unmount()) container.remove() + vi.useRealTimers() }) + it.each(['mothership', 'workflow', 'browser'])( + 'shows one model-described action without a redundant %s disclosure', + (agentName) => { + act(() => + root.render( + createElement(AgentGroup, { + agentName, + agentLabel: agentName, + defaultExpanded: true, + isLaneOpen: true, + isStreaming: true, + items: [ + { + type: 'tool', + data: { + id: 'described-read', + toolName: 'read', + displayTitle: 'Reading files', + activityDescription: 'Checking the project timeline', + status: 'executing', + }, + }, + ], + }) + ) + ) + + const statuses = [...container.querySelectorAll('[role="status"]')] + expect(statuses).toHaveLength(1) + expect(container.querySelector('[role="button"]')).toBeNull() + for (const status of statuses) { + expect(status.textContent).toContain('Checking the project timeline') + } + expect(container.textContent).not.toContain('Reading files') + expect(container.querySelector('[class*="shimmer"]')).not.toBeNull() + } + ) + it.each([ ['executing', 'Reading notes'], ['success', 'Read notes'], @@ -151,12 +190,13 @@ describe('AgentGroup inline main activity', () => { ) expect(container.textContent).toBe(expected) expect(container.querySelectorAll('[role="status"]')).toHaveLength(1) - expect(container.querySelector('button')).toBeNull() + expect(container.querySelector('[role="button"]')).toBeNull() expect(container.querySelector('[data-state]')).toBeNull() expect(Boolean(container.querySelector('[class*="shimmer"]'))).toBe(status === 'executing') }) - it('replaces the active status in place and expands the full completed history', () => { + it('paces the active status in place and expands the full completed history', () => { + vi.useFakeTimers() const first: AgentGroupItem = { type: 'tool', data: { id: 'first', toolName: 'grep', displayTitle: 'Searching files', status: 'executing' }, @@ -180,14 +220,18 @@ describe('AgentGroup inline main activity', () => { render([first]) expect(container.textContent).toBe('Searching files') - expect(container.querySelector('button')).toBeNull() + expect(container.querySelector('[role="button"]')).toBeNull() const activity = container.firstElementChild render([first, next]) expect(container.firstElementChild).toBe(activity) + expect(container.textContent).toBe('Searching files') + act(() => vi.advanceTimersByTime(1000)) expect(container.textContent).toBe('Reading notes') expect(container.querySelector('[class*="shimmer"]')).not.toBeNull() - expect(container.querySelector('button')?.getAttribute('aria-expanded')).toBe('false') + expect( + container.querySelector('[role="button"]')?.getAttribute('aria-expanded') + ).toBe('false') expect(container.querySelector('svg')).not.toBeNull() expect(container.textContent).not.toContain('Sim') @@ -200,7 +244,7 @@ describe('AgentGroup inline main activity', () => { ) expect(container.textContent).toBe('Searched files, read files') expect(container.querySelector('[class*="shimmer"]')).toBeNull() - const header = container.querySelector('button') + const header = container.querySelector('[role="button"]') act(() => header?.click()) expect(header?.getAttribute('aria-expanded')).toBe('true') expect(container.querySelector('[data-state="open"]')?.textContent).toBe( @@ -237,7 +281,7 @@ describe('AgentGroup inline main activity', () => { ) ) render([first, second]) - act(() => container.querySelector('button')?.click()) + act(() => container.querySelector('[role="button"]')?.click()) render([ first, second, @@ -251,7 +295,9 @@ describe('AgentGroup inline main activity', () => { }, }, ]) - expect(container.querySelector('button')?.getAttribute('aria-expanded')).toBe('true') + expect( + container.querySelector('[role="button"]')?.getAttribute('aria-expanded') + ).toBe('true') expect(container.querySelector('[data-state="open"]')?.textContent).toBe( 'Read notesRead more notesRunning checks' ) @@ -290,15 +336,15 @@ describe('AgentGroup inline main activity', () => { render([wait]) act(() => vi.advanceTimersByTime(2000)) expect(container.textContent).toBe('Waiting 1s') - expect(container.querySelector('button')).toBeNull() + expect(container.querySelector('[role="button"]')).toBeNull() render([wait, read]) expect(container.textContent).toBe('Waiting 1s') expect(setIntervalSpy).toHaveBeenCalledTimes(1) - const header = container.querySelector('button') + const header = container.querySelector('[role="button"]') act(() => header?.click()) expect(header?.hasAttribute('aria-label')).toBe(false) - expect(header?.textContent).toBe('Waiting 1s') - expect(header).toHaveAccessibleName('Waiting 1s') + expect(header?.textContent).toBe('Tool activity') + expect(header).toHaveAccessibleName('Tool activity') expect(container.querySelector('[data-state="open"]')?.textContent).toBe( 'Waiting 1sRead notes' ) @@ -314,8 +360,8 @@ describe('AgentGroup inline main activity', () => { read, { ...wait, data: { ...wait.data, id: 'wait-second' } }, ]) - expect(header?.textContent).toBe('Waiting 3s') - expect(header).toHaveAccessibleName('Waiting 3s') + expect(header?.textContent).toBe('Tool activity') + expect(header).toHaveAccessibleName('Tool activity') expect(container.querySelector('.overflow-y-auto')).toBe(viewport) expect(container.querySelector('[data-state="open"]')?.textContent).toBe( 'WaitedRead notesWaiting 3s' @@ -367,9 +413,9 @@ describe('AgentGroup inline main activity', () => { }) ) ) - const header = container.querySelector('button') - expect(header?.textContent).toBe('Agent — Read files, ran commands') - expect(header).toHaveAccessibleName('Agent — Read files, ran commands') + const header = container.querySelector('[role="button"]') + expect(header?.textContent).toBe('Read files, ran commands') + expect(header).toHaveAccessibleName('Read files, ran commands') expect(container.querySelectorAll('[data-tool-call-id]')).toHaveLength(0) act(() => header?.click()) expect( @@ -406,12 +452,19 @@ describe('AgentGroup inline main activity', () => { ], ToolCallComponent: ({ toolCallId, displayTitle, renderStatus }: ToolCallItemProps) => { const status = createElement('div', { 'data-tool-call-id': toolCallId }, displayTitle) - return renderStatus ? renderStatus(status) : status + return renderStatus + ? renderStatus({ + label: displayTitle, + activeLabel: displayTitle, + isActive: true, + icon: createElement('svg', { 'data-tool-call-id': toolCallId }), + }) + : status }, }) ) ) - const headers = Array.from(container.querySelectorAll('button')) + const headers = Array.from(container.querySelectorAll('[role="button"]')) expect(headers).toHaveLength(2) expect(headers.every((header) => header.getAttribute('aria-expanded') === 'true')).toBe(true) act(() => headers[0].click()) @@ -512,7 +565,14 @@ describe('AgentGroup inline main activity', () => { isStreaming: true, ToolCallComponent: ({ toolCallId, displayTitle, renderStatus }: ToolCallItemProps) => { const status = createElement('div', { 'data-tool-call-id': toolCallId }, displayTitle) - return renderStatus ? renderStatus(status) : status + return renderStatus + ? renderStatus({ + label: displayTitle, + activeLabel: displayTitle, + isActive: true, + icon: createElement('svg', { 'data-tool-call-id': toolCallId }), + }) + : status }, }) ) @@ -559,8 +619,8 @@ describe('AgentGroup browser takeover', () => { expect(liftedQuestion).toBeDefined() expect(collapsedLog?.contains(liftedQuestion ?? null)).toBe(false) - const header = Array.from(container.querySelectorAll('button')).find((button) => - button.textContent?.includes('Browser Agent') + const header = Array.from(container.querySelectorAll('[role="button"]')).find( + (button) => button.hasAttribute('aria-expanded') ) act(() => header?.click()) expect(container.querySelector('[data-state="open"]')).not.toBeNull() @@ -648,7 +708,7 @@ describe('AgentGroup browser takeover', () => { expect(container.querySelector('.animate-stream-fade-in')).toBeNull() // Groups never auto-expand: the answered question lives inside the // collapsed log until the user opens it manually. - const headerToggle = container.querySelector('button[class*="group/agent"]') + const headerToggle = container.querySelector('[role="button"][class*="group/agent"]') expect(headerToggle).not.toBeNull() act(() => { headerToggle?.dispatchEvent(new MouseEvent('click', { bubbles: true })) @@ -713,10 +773,10 @@ describe('AgentGroup nested status line', () => { namedTool('Reading workflow', 'success' as ToolCallStatus, 1), group([namedTool('Deploying Invoice Sync as API', 'executing' as ToolCallStatus, 2)]), ]) - expect(header).toContain('Workflow Agent — Deploying Invoice Sync as API') + expect(header).toContain('Deploying Invoice Sync as API') }) - it('counts running tools across depths with the + n suffix', () => { + it('selects the latest running tool across depths', () => { const header = render([ namedTool('Reading workflow', 'executing' as ToolCallStatus, 1), group([ @@ -724,8 +784,8 @@ describe('AgentGroup nested status line', () => { namedTool('Checking deployment status', 'executing' as ToolCallStatus, 2), ]), ]) - // Latest start wins; the other two running become the overflow count. - expect(header).toContain('Deploying Invoice Sync as API + 2') + /** The latest start wins across the subtree. */ + expect(header).toContain('Deploying Invoice Sync as API') }) it('falls back to the last tool at any depth when nothing is running', () => { @@ -733,6 +793,6 @@ describe('AgentGroup nested status line', () => { namedTool('Reading workflow', 'success' as ToolCallStatus, 1), group([namedTool('Deploying Invoice Sync as API', 'success' as ToolCallStatus, 2)]), ]) - expect(header).toContain('Workflow Agent — Deploying Invoice Sync as API') + expect(header).toContain('Deploying Invoice Sync as API') }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/browser-agent-icon.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/browser-agent-icon.test.tsx index 70b38e2bf03..13ee89ed18f 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/browser-agent-icon.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/browser-agent-icon.test.tsx @@ -4,6 +4,7 @@ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { AgentGroup } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group' import type { AgentGroupItem } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view' import { BrowserAgentIcon, @@ -227,6 +228,58 @@ describe('BrowserAgentIcon', () => { ) } + it('keeps a loaded favicon paired with the paced label when disclosure appears and the site changes', () => { + vi.useFakeTimers() + const first = tool({ + id: 'first', + displayTitle: 'Opening first page', + result: undefined, + status: 'executing', + params: { url: 'https://example.com/' }, + }) + const second = tool({ + id: 'second', + displayTitle: 'Opening second page', + result: undefined, + status: 'executing', + params: { url: 'https://example.org/' }, + }) + const renderGroup = (items: AgentGroupItem[]) => + act(() => + root.render( + + ) + ) + try { + openPage('https://example.com/') + renderGroup([first]) + const img = container.querySelector('img')! + act(() => img.dispatchEvent(new Event('load'))) + act(() => vi.advanceTimersByTime(100)) + openPage('https://example.org/') + renderGroup([first, second]) + expect(container.querySelector('img')).toBe(img) + expect(container.querySelector('[role="status"]')?.textContent).toBe('Opening first page') + expect(container.querySelector('[role="button"]')).not.toBeNull() + act(() => vi.advanceTimersByTime(900)) + expect(container.querySelector('[role="status"]')?.textContent).toBe('Opening second page') + const nextImage = container.querySelector('img')! + expect(nextImage).not.toBe(img) + expect(nextImage.src).toBe('https://example.org/favicon.ico') + act(() => nextImage.dispatchEvent(new Event('error'))) + expect(container.querySelector('img')).toBeNull() + expect(container.querySelector('[role="status"] svg')).not.toBeNull() + } finally { + vi.useRealTimers() + } + }) + it('does not contact sites from history, other chats, or pending navigation', () => { render('https://example.com/document') expect(container.querySelector('img')).toBeNull() diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/main-agent-activity.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/main-agent-activity.tsx index 82e62b4e3a4..140ddea8e48 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/main-agent-activity.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/main-agent-activity.tsx @@ -11,6 +11,7 @@ interface MainAgentActivityProps { ToolCallComponent: ComponentType renderItem: (item: AgentGroupItem, index: number) => ReactNode autoScrollActivity: boolean + isActive: boolean } /** Keep answers and interactions in the transcript, outside collapsible tool history. */ @@ -27,15 +28,17 @@ export function MainAgentActivity({ ToolCallComponent, renderItem, autoScrollActivity, + isActive, }: MainAgentActivityProps) { const activity: ReactNode[] = [] let tools: ToolCallData[] = [] - const flushTools = () => { + const flushTools = (active = false) => { if (tools.length === 0) return activity.push( @@ -63,7 +66,7 @@ export function MainAgentActivity({ ) } - flushTools() + flushTools(isActive) return
{activity}
} diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts index e0b17ababbf..e1c23b42d1b 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts @@ -24,7 +24,18 @@ describe('getToolActivitySummary', () => { tool('browser_type'), tool('browser_navigate'), ]) - ).toBe('Navigated pages, read pages +1 more') + ).toBe('Navigated, read pages +1 more') + }) + + it.each([ + [['browser_navigate', 'browser_read_text'], 'Navigated, read pages'], + [['browser_read_text', 'browser_navigate'], 'Read, navigated pages'], + [['browser_navigate', 'browser_read_text', 'browser_scroll'], 'Navigated, read pages +1 more'], + [['browser_navigate', 'browser_type'], 'Navigated pages, entered text'], + [['browser_navigate', 'browser_navigate'], 'Navigated pages'], + [['read', 'browser_read_text'], 'Read files, read pages'], + ])('compacts only explicit shared objects: %j', (names, expected) => { + expect(getToolActivitySummary((names as string[]).map((name) => tool(name)))).toBe(expected) }) it('does not describe unsuccessful work as completed actions', () => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.tsx index d426fe587da..11b95a3833e 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.tsx @@ -2,10 +2,11 @@ import { type ComponentType, Fragment, useState } from 'react' import { ActivityStatus } from '@/components/ui/activity-status' -import { getToolActivityLabel } from '@/lib/copilot/tools/tool-activity' +import { getToolActivitySummaryActions } from '@/lib/copilot/tools/tool-activity' import { getToolStatusDisplayTitle } from '@/lib/copilot/tools/tool-display' -import { ActivityDisclosure } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-disclosure' +import { ActivityStream } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream' import type { ToolCallItemProps } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item' +import { getActivityAttentionKey } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-interactions' import { getToolIcon } from '@/app/workspace/[workspaceId]/home/components/message-content/utils' import { type ToolCallData, ToolCallStatus } from '@/app/workspace/[workspaceId]/home/types' @@ -15,94 +16,126 @@ const MAX_SUMMARY_ACTIONS = 2 export function getToolActivitySummary(tools: ToolCallData[]): string { if (tools.length === 1) { const tool = tools[0] - return getToolStatusDisplayTitle(tool.displayTitle, tool.status, tool.toolName) + return getToolStatusDisplayTitle( + tool.displayTitle, + tool.status, + tool.toolName, + tool.activityDescription + ) } - const labels = new Set() + const { labels, additionalActions } = getToolActivitySummaryActions( + tools.filter((tool) => tool.status === ToolCallStatus.success), + MAX_SUMMARY_ACTIONS + ) + const summary = labels.join(', ') + const summaryLabel = summary ? summary[0].toUpperCase() + summary.slice(1) : 'Tool activity' + return [ + additionalActions > 0 ? `${summaryLabel} +${additionalActions} more` : summaryLabel, + ...getToolActivityOutcomes(tools), + ].join(' · ') +} + +function getToolActivityOutcomes(tools: ToolCallData[]): string[] { let failed = 0 let stopped = 0 let skipped = 0 for (const tool of tools) { - if (tool.status === ToolCallStatus.success) { - labels.add(getToolActivityLabel(tool.toolName, tool.params)) - } else if (tool.status === ToolCallStatus.error || tool.status === ToolCallStatus.rejected) - failed++ + if (tool.status === ToolCallStatus.error || tool.status === ToolCallStatus.rejected) failed++ else if (tool.status === ToolCallStatus.cancelled || tool.status === ToolCallStatus.interrupted) stopped++ else if (tool.status === ToolCallStatus.skipped) skipped++ } - const summary = Array.from(labels).slice(0, MAX_SUMMARY_ACTIONS).join(', ') - const summaryLabel = summary ? summary[0].toUpperCase() + summary.slice(1) : 'Tool activity' - const additionalActions = Math.max(0, labels.size - MAX_SUMMARY_ACTIONS) - const outcomes = [ - failed && `${failed} failed`, - stopped && `${stopped} stopped`, - skipped && `${skipped} skipped`, - ].filter(Boolean) return [ - additionalActions > 0 ? `${summaryLabel} +${additionalActions} more` : summaryLabel, - ...outcomes, - ].join(' · ') + ...(failed ? [`${failed} failed`] : []), + ...(stopped ? [`${stopped} stopped`] : []), + ...(skipped ? [`${skipped} skipped`] : []), + ] +} + +/** Keep earlier parallel failures visible while the latest action continues. */ +export function getActiveToolActivityTitle( + label: string, + tool: ToolCallData, + tools: ToolCallData[] +): string { + return tool.status === ToolCallStatus.executing || tool.status === ToolCallStatus.success + ? [label, ...getToolActivityOutcomes(tools)].join(' · ') + : label +} + +/** Keep running work visible until every parallel call finishes. */ +export function getActivityStatusTool(tools: ToolCallData[]): ToolCallData | undefined { + return ( + tools.reduce( + (newest, tool) => + tool.status === ToolCallStatus.executing && + (!newest || (tool.startedAt ?? 0) >= (newest.startedAt ?? 0)) + ? tool + : newest, + undefined + ) ?? tools.at(-1) + ) } interface ToolActivityGroupProps { tools: ToolCallData[] ToolCallComponent: ComponentType autoScrollActivity?: boolean + isActive?: boolean } export function ToolActivityGroup({ tools, ToolCallComponent, autoScrollActivity = true, + isActive = false, }: ToolActivityGroupProps) { const [expanded, setExpanded] = useState(false) - let activeTool: ToolCallData | undefined - for (let index = tools.length - 1; index >= 0; index--) { - if (tools[index].status === ToolCallStatus.executing) { - activeTool = tools[index] - break - } - } - const statusTool = activeTool ?? tools[tools.length - 1] + const statusTool = getActivityStatusTool(tools) + if (!statusTool) return null + const working = isActive || tools.some((tool) => tool.status === ToolCallStatus.executing) + const headerActive = + working && + (statusTool.status === ToolCallStatus.executing || statusTool.status === ToolCallStatus.success) + const attentionKey = getActivityAttentionKey(tools) const SummaryIcon = getToolIcon(tools[0].toolName) return ( { - if (tools.length === 1) return status - return ( - } - /> - ) - } - expanded={expanded} - onToggle={() => setExpanded(!expanded)} - isStreaming={Boolean(activeTool) && autoScrollActivity} - > -
- {tools.map((tool) => ( - - {tool.id === statusTool.id ? ( - status - ) : ( - - )} - - ))} -
- - ) - }} + renderStatus={(status) => ( + , + }} + activityKey={statusTool.id} + attentionKey={attentionKey} + collapsible={tools.length > 1} + expanded={expanded} + onToggle={() => setExpanded(!expanded)} + isStreaming={working && autoScrollActivity} + > +
+ {tools.map((tool) => ( + + {tool.id === statusTool.id ? ( + + ) : ( + + )} + + ))} +
+ + )} /> ) } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx index b52d7b1b0e6..1692702faec 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx @@ -5,9 +5,11 @@ import { act, type ReactNode, type SVGProps } from 'react' import { createRoot, type Root } from 'react-dom/client' import { renderToStaticMarkup } from 'react-dom/server' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { ToolActivityGroup } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group' +import { ToolCallItem } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item' +import type { ToolCallData } from '@/app/workspace/[workspaceId]/home/types' import { notifyBlockOverlayChanged } from '@/blocks/custom/client-overlay' import { getBlock, getBlockByToolName } from '@/blocks/registry' -import { ToolCallItem } from './tool-call-item' vi.mock('@/components/ui', () => ({ ShimmerText: ({ children }: { children: ReactNode }) => {children}, @@ -48,6 +50,90 @@ describe('ToolCallItem', () => { expect(markup).not.toContain('Writing brief.md') }) + it.each([ + ['executing', 'Checking the invoice totals'], + ['success', 'Checked the invoice totals'], + ['error', 'Failed checking the invoice totals'], + ['cancelled', 'Stopped checking the invoice totals'], + ['rejected', 'Failed checking the invoice totals'], + ['skipped', 'Skipped checking the invoice totals'], + ] as const)( + 'projects %s from the actual tool status onto the model description', + (status, title) => { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain(title) + expect(markup).not.toContain('report.md') + } + ) + + it.each([' ', 'a'.repeat(161)])( + 'uses the existing title for an invalid description', + (activityDescription) => { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('Searching files') + } + ) + + it('keeps an executing wait countdown in place of the model phrase', () => { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('10s') + expect(markup).not.toContain('Waiting for the export') + }) + + it('renders model descriptions as text, without interpreting markup', () => { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('<script>') + expect(markup).not.toContain('