From d41bb5c55dd932e16306c00431b5b02dc40d9558 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 9 Jul 2026 12:50:47 -0400 Subject: [PATCH 1/9] fix(devices): show reconnect hint when device-sync connection is errored When an org's only device-sync-capable connection (Intune/Kandji/JumpCloud) is in error state (e.g. expired OAuth), the Devices tab hid the sync selector entirely with no explanation. Now available-providers reports connectionStatus ('active' | 'error' | null) and the selector renders a reconnect hint linking to Integrations instead of rendering nothing. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc --- ...ync-available-providers.controller.spec.ts | 162 ++++++++++++++++++ .../controllers/sync.controller.ts | 19 ++ .../DeviceSyncProviderSelector.test.tsx | 90 ++++++++++ .../components/DeviceSyncProviderSelector.tsx | 26 ++- .../people/devices/hooks/useDeviceSync.ts | 6 + 5 files changed, 302 insertions(+), 1 deletion(-) create mode 100644 apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts diff --git a/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts b/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts new file mode 100644 index 0000000000..6782e6322d --- /dev/null +++ b/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts @@ -0,0 +1,162 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { SyncController } from './sync.controller'; +import { HybridAuthGuard } from '../../auth/hybrid-auth.guard'; +import { PermissionGuard } from '../../auth/permission.guard'; +import { ConnectionRepository } from '../repositories/connection.repository'; +import { CredentialVaultService } from '../services/credential-vault.service'; +import { OAuthCredentialsService } from '../services/oauth-credentials.service'; +import { IntegrationSyncLoggerService } from '../services/integration-sync-logger.service'; +import { GenericEmployeeSyncService } from '../services/generic-employee-sync.service'; +import { GenericDeviceSyncService } from '../services/generic-device-sync.service'; +import { DynamicIntegrationRepository } from '../repositories/dynamic-integration.repository'; +import { CheckRunRepository } from '../repositories/check-run.repository'; + +const mockConnectionFindFirst = jest.fn(); +const mockGetActiveManifests = jest.fn(); + +jest.mock('@db', () => ({ + db: { + integrationConnection: { + findFirst: (...args: unknown[]) => mockConnectionFindFirst(...args), + }, + }, +})); + +jest.mock('../../auth/auth.server', () => ({ + auth: { api: { getSession: jest.fn() } }, +})); + +jest.mock('@trycompai/auth', () => ({ + statement: { integration: ['create', 'read', 'update', 'delete'] }, + BUILT_IN_ROLE_PERMISSIONS: {}, +})); + +jest.mock('@trycompai/integration-platform', () => { + const actual = jest.requireActual< + typeof import('@trycompai/integration-platform') + >('@trycompai/integration-platform'); + return { + ...actual, + registry: { + getActiveManifests: (...args: unknown[]) => + mockGetActiveManifests(...args), + }, + TASK_TEMPLATE_INFO: {}, + }; +}); + +describe('SyncController - getAvailableSyncProviders connection status', () => { + let controller: SyncController; + + const orgId = 'org_test123'; + const intuneManifest = { + id: 'intune', + name: 'Intune', + logoUrl: 'https://example.com/intune.png', + capabilities: ['checks', 'device_sync'], + }; + + beforeEach(async () => { + jest.clearAllMocks(); + mockGetActiveManifests.mockReturnValue([intuneManifest]); + + const module: TestingModule = await Test.createTestingModule({ + controllers: [SyncController], + providers: [ + { + provide: ConnectionRepository, + useValue: { findById: jest.fn(), update: jest.fn() }, + }, + { + provide: CredentialVaultService, + useValue: { + getDecryptedCredentials: jest.fn(), + refreshOAuthTokens: jest.fn(), + }, + }, + { + provide: OAuthCredentialsService, + useValue: { getCredentials: jest.fn() }, + }, + { + provide: IntegrationSyncLoggerService, + useValue: { logSync: jest.fn() }, + }, + { provide: GenericEmployeeSyncService, useValue: {} }, + { provide: GenericDeviceSyncService, useValue: {} }, + { provide: DynamicIntegrationRepository, useValue: {} }, + { + provide: CheckRunRepository, + useValue: { create: jest.fn(), complete: jest.fn() }, + }, + ], + }) + .overrideGuard(HybridAuthGuard) + .useValue({ canActivate: () => true }) + .overrideGuard(PermissionGuard) + .useValue({ canActivate: () => true }) + .compile(); + + controller = module.get(SyncController); + }); + + it('reports connected + connectionStatus active for an active connection', async () => { + mockConnectionFindFirst.mockImplementation( + (args: { where: { status: string } }) => + args.where.status === 'active' + ? { + id: 'icn_active', + status: 'active', + lastSyncAt: null, + nextSyncAt: null, + } + : null, + ); + + const result = await controller.getAvailableSyncProviders(orgId, 'device'); + + expect(result.providers).toEqual([ + expect.objectContaining({ + slug: 'intune', + connected: true, + connectionStatus: 'active', + connectionId: 'icn_active', + }), + ]); + // With an active connection there is no need to look up an errored one. + expect(mockConnectionFindFirst).toHaveBeenCalledTimes(1); + }); + + it('reports connectionStatus error (not connected) when the only connection is broken', async () => { + mockConnectionFindFirst.mockImplementation( + (args: { where: { status: string } }) => + args.where.status === 'error' ? { id: 'icn_broken' } : null, + ); + + const result = await controller.getAvailableSyncProviders(orgId, 'device'); + + expect(result.providers).toEqual([ + expect.objectContaining({ + slug: 'intune', + connected: false, + connectionStatus: 'error', + connectionId: null, + }), + ]); + }); + + it('reports connectionStatus null when the org has no connection for the provider', async () => { + mockConnectionFindFirst.mockResolvedValue(null); + + const result = await controller.getAvailableSyncProviders(orgId, 'device'); + + expect(result.providers).toEqual([ + expect.objectContaining({ + slug: 'intune', + connected: false, + connectionStatus: null, + connectionId: null, + }), + ]); + }); +}); diff --git a/apps/api/src/integration-platform/controllers/sync.controller.ts b/apps/api/src/integration-platform/controllers/sync.controller.ts index 5386f0fd28..4412e6b64a 100644 --- a/apps/api/src/integration-platform/controllers/sync.controller.ts +++ b/apps/api/src/integration-platform/controllers/sync.controller.ts @@ -1792,11 +1792,30 @@ export class SyncController { nextSyncAt: true, }, }); + // No active connection: surface a broken (errored) one so the UI can + // prompt a reconnect instead of silently hiding sync for the provider. + // Deliberately ignores 'disconnected' — the user chose to disconnect. + const erroredConnection = connection + ? null + : await db.integrationConnection.findFirst({ + where: { + organizationId, + status: 'error', + provider: { slug: m.id }, + }, + select: { id: true }, + orderBy: { updatedAt: 'desc' }, + }); return { slug: m.id, name: m.name, logoUrl: m.logoUrl, connected: !!connection, + connectionStatus: connection + ? ('active' as const) + : erroredConnection + ? ('error' as const) + : null, connectionId: connection?.id ?? null, lastSyncAt: connection?.lastSyncAt?.toISOString() ?? null, nextSyncAt: connection?.nextSyncAt?.toISOString() ?? null, diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx index 4366fe3d35..534e36413e 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx @@ -115,3 +115,93 @@ describe('DeviceSyncProviderSelector — RBAC gating', () => { expect(container).toBeEmptyDOMElement(); }); }); + +describe('DeviceSyncProviderSelector — errored connection hint', () => { + beforeEach(() => { + mockHasPermission.mockImplementation( + (resource: string, action: string) => + resource === 'integration' && action === 'update', + ); + }); + + it('shows a reconnect hint when the only connection is in error state', () => { + mockUseDeviceSync.mockReturnValue({ + selectedProvider: null, + isSyncing: false, + isLoading: false, + availableProviders: [ + { + ...provider, + slug: 'intune', + name: 'Intune', + connected: false, + connectionStatus: 'error', + connectionId: null, + }, + ], + syncDevices: vi.fn(), + setSyncProvider: vi.fn(), + getProviderName: (slug: string) => slug, + getProviderLogo: () => '', + hasAnyConnection: false, + }); + + render(); + + expect( + screen.getByText(/the Intune connection needs to be reconnected/i), + ).toBeInTheDocument(); + expect( + screen.getByRole('link', { name: /Go to Integrations/i }), + ).toHaveAttribute('href', '/org_1/integrations'); + // No sync controls without an active connection. + expect( + screen.queryByRole('button', { name: /Sync now/i }), + ).not.toBeInTheDocument(); + }); + + it('renders nothing when there is no connection at all (no errored one either)', () => { + mockUseDeviceSync.mockReturnValue({ + selectedProvider: null, + isSyncing: false, + isLoading: false, + availableProviders: [ + { + ...provider, + connected: false, + connectionStatus: null, + connectionId: null, + }, + ], + syncDevices: vi.fn(), + setSyncProvider: vi.fn(), + getProviderName: (slug: string) => slug, + getProviderLogo: () => '', + hasAnyConnection: false, + }); + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); + + it('renders nothing when the API omits connectionStatus (older API response)', () => { + mockUseDeviceSync.mockReturnValue({ + selectedProvider: null, + isSyncing: false, + isLoading: false, + availableProviders: [ + { ...provider, connected: false, connectionId: null }, + ], + syncDevices: vi.fn(), + setSyncProvider: vi.fn(), + getProviderName: (slug: string) => slug, + getProviderLogo: () => '', + hasAnyConnection: false, + }); + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx index 1d899a7019..1349d22c7b 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx @@ -1,5 +1,6 @@ 'use client'; +import Link from 'next/link'; import { useParams } from 'next/navigation'; import { Button, Skeleton } from '@trycompai/design-system'; import { Renew } from '@trycompai/design-system/icons'; @@ -34,7 +35,30 @@ export function DeviceSyncProviderSelector() { } if (!hasAnyConnection) { - return null; + // No active connection — but if one exists in an error state (e.g. expired + // OAuth), say so instead of hiding device sync entirely, so the user knows + // a reconnect brings it back. + const erroredProviders = availableProviders.filter( + (p) => p.connectionStatus === 'error', + ); + if (erroredProviders.length === 0) { + return null; + } + const names = erroredProviders.map((p) => p.name).join(', '); + return ( +
+
+ Device sync is unavailable — the {names} connection + {erroredProviders.length > 1 ? 's need' : ' needs'} to be reconnected. +
+ + Go to Integrations + +
+ ); } const connectedProviders = availableProviders.filter((p) => p.connected); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/hooks/useDeviceSync.ts b/apps/app/src/app/(app)/[orgId]/people/devices/hooks/useDeviceSync.ts index 20b4e8e532..54cca80f1e 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/hooks/useDeviceSync.ts +++ b/apps/app/src/app/(app)/[orgId]/people/devices/hooks/useDeviceSync.ts @@ -10,6 +10,12 @@ export interface DeviceSyncProviderInfo { name: string; logoUrl: string; connected: boolean; + /** + * 'error' means the org has a connection for this provider but it is broken + * (e.g. expired OAuth) and needs a reconnect. Optional so the UI tolerates + * API responses from before this field existed. + */ + connectionStatus?: 'active' | 'error' | null; connectionId: string | null; lastSyncAt: string | null; nextSyncAt: string | null; From 8e4a9d41b8c803cba15ea17c955dad905e6d7986 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 9 Jul 2026 13:37:41 -0400 Subject: [PATCH 2/9] refactor(devices): device-sync selector follows the People sync-source pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the notification-bar approach with the established sync-source control UX (TwoFactorSourceSelector / people sync): an always-visible labeled select — dashed 'Connect an integration' slot when the org has no device-sync connection, provider options with logo/Active marker and a 'Don't auto-sync' option, and broken (errored) connections listed as disabled options marked 'Reconnect'. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc --- .../DeviceSyncProviderSelector.test.tsx | 130 ++++----- .../components/DeviceSyncProviderSelector.tsx | 252 +++++++++++------- 2 files changed, 224 insertions(+), 158 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx index 534e36413e..e95615ed0a 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx @@ -1,4 +1,5 @@ import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { DeviceSyncProviderSelector } from './DeviceSyncProviderSelector'; import type { DeviceSyncProviderInfo } from '../hooks/useDeviceSync'; @@ -26,15 +27,21 @@ const provider: DeviceSyncProviderInfo = { name: 'Jamf', logoUrl: 'https://example.com/jamf.png', connected: true, + connectionStatus: 'active', connectionId: 'icn_1', lastSyncAt: null, nextSyncAt: null, }; -beforeEach(() => { - vi.clearAllMocks(); - mockUseDeviceSync.mockReturnValue({ - selectedProvider: 'jamf', +function mockHook( + overrides: Partial> = {}, +) { + mockUseDeviceSync.mockReturnValue({ ...buildHookReturn(), ...overrides }); +} + +function buildHookReturn() { + return { + selectedProvider: 'jamf' as string | null, isSyncing: false, isLoading: false, availableProviders: [provider], @@ -43,7 +50,12 @@ beforeEach(() => { getProviderName: (slug: string) => (slug === 'jamf' ? 'Jamf' : slug), getProviderLogo: () => provider.logoUrl, hasAnyConnection: true, - }); + }; +} + +beforeEach(() => { + vi.clearAllMocks(); + mockHook(); }); describe('DeviceSyncProviderSelector — RBAC gating', () => { @@ -55,10 +67,14 @@ describe('DeviceSyncProviderSelector — RBAC gating', () => { render(); + expect( + screen.getByRole('combobox', { name: /Sync devices from/i }), + ).toBeInTheDocument(); expect( screen.getByRole('button', { name: /Sync now/i }), ).toBeInTheDocument(); - expect(screen.getByText('Jamf')).toBeInTheDocument(); + // Trigger (and the inline option list) show the selected provider name. + expect(screen.getAllByText('Jamf').length).toBeGreaterThan(0); // Hook is enabled (and therefore allowed to hit the device-sync APIs). expect(mockUseDeviceSync).toHaveBeenCalledWith( expect.objectContaining({ enabled: true }), @@ -80,28 +96,30 @@ describe('DeviceSyncProviderSelector — RBAC gating', () => { ); }); - it('shows the provider picker when the saved provider is no longer connected', () => { + it('shows the provider picker when the saved provider is no longer connected', async () => { + const user = userEvent.setup(); mockHasPermission.mockImplementation( (resource: string, action: string) => resource === 'integration' && action === 'update', ); - mockUseDeviceSync.mockReturnValue({ + mockHook({ selectedProvider: 'jamf', // saved, but no longer in the connected list - isSyncing: false, - isLoading: false, availableProviders: [{ ...provider, slug: 'kandji', name: 'Kandji' }], - syncDevices: vi.fn(), - setSyncProvider: vi.fn(), - getProviderName: (slug: string) => (slug === 'kandji' ? 'Kandji' : slug), - getProviderLogo: () => provider.logoUrl, - hasAnyConnection: true, }); render(); // The picker must be available so the user can switch to a connected provider. - expect(screen.getByRole('combobox')).toBeInTheDocument(); - expect(screen.getByRole('option', { name: 'Kandji' })).toBeInTheDocument(); + const trigger = screen.getByRole('combobox', { name: /Sync devices from/i }); + expect(screen.getByText('Not syncing')).toBeInTheDocument(); + await user.click(trigger); + expect( + screen.getByRole('option', { name: /Kandji/i }), + ).toBeInTheDocument(); + // No Sync now button without a connected selected provider. + expect( + screen.queryByRole('button', { name: /Sync now/i }), + ).not.toBeInTheDocument(); }); it('does not render for read-only integration access (integration:read only)', () => { @@ -116,7 +134,7 @@ describe('DeviceSyncProviderSelector — RBAC gating', () => { }); }); -describe('DeviceSyncProviderSelector — errored connection hint', () => { +describe('DeviceSyncProviderSelector — connection states', () => { beforeEach(() => { mockHasPermission.mockImplementation( (resource: string, action: string) => @@ -124,84 +142,70 @@ describe('DeviceSyncProviderSelector — errored connection hint', () => { ); }); - it('shows a reconnect hint when the only connection is in error state', () => { - mockUseDeviceSync.mockReturnValue({ + it('shows a labeled connect slot when no device-sync integration has a connection', () => { + mockHook({ selectedProvider: null, - isSyncing: false, - isLoading: false, availableProviders: [ - { - ...provider, - slug: 'intune', - name: 'Intune', - connected: false, - connectionStatus: 'error', - connectionId: null, - }, + { ...provider, connected: false, connectionStatus: null, connectionId: null }, ], - syncDevices: vi.fn(), - setSyncProvider: vi.fn(), - getProviderName: (slug: string) => slug, - getProviderLogo: () => '', hasAnyConnection: false, }); render(); + expect(screen.getByText('Device sync')).toBeInTheDocument(); expect( - screen.getByText(/the Intune connection needs to be reconnected/i), - ).toBeInTheDocument(); - expect( - screen.getByRole('link', { name: /Go to Integrations/i }), + screen.getByRole('link', { name: /Connect an integration/i }), ).toHaveAttribute('href', '/org_1/integrations'); - // No sync controls without an active connection. - expect( - screen.queryByRole('button', { name: /Sync now/i }), - ).not.toBeInTheDocument(); + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); }); - it('renders nothing when there is no connection at all (no errored one either)', () => { - mockUseDeviceSync.mockReturnValue({ + it('lists a broken connection as a disabled option marked Reconnect', async () => { + const user = userEvent.setup(); + mockHook({ selectedProvider: null, - isSyncing: false, - isLoading: false, availableProviders: [ { ...provider, + slug: 'intune', + name: 'Intune', connected: false, - connectionStatus: null, + connectionStatus: 'error', connectionId: null, }, ], - syncDevices: vi.fn(), - setSyncProvider: vi.fn(), - getProviderName: (slug: string) => slug, - getProviderLogo: () => '', hasAnyConnection: false, }); - const { container } = render(); + render(); - expect(container).toBeEmptyDOMElement(); + // The select renders (not the connect slot): the org HAS a connection, + // it just needs a reconnect. + const trigger = screen.getByRole('combobox', { name: /Sync devices from/i }); + await user.click(trigger); + const intuneOption = await screen.findByRole('option', { name: /Intune/i }); + expect(intuneOption).toHaveAttribute('aria-disabled', 'true'); + expect(screen.getByText('Reconnect')).toBeInTheDocument(); + // Not selectable as a sync source, so no Sync now button either. + expect( + screen.queryByRole('button', { name: /Sync now/i }), + ).not.toBeInTheDocument(); }); - it('renders nothing when the API omits connectionStatus (older API response)', () => { - mockUseDeviceSync.mockReturnValue({ + it('falls back to the connect slot when the API omits connectionStatus (older API response)', () => { + mockHook({ selectedProvider: null, - isSyncing: false, - isLoading: false, availableProviders: [ { ...provider, connected: false, connectionId: null }, ], - syncDevices: vi.fn(), - setSyncProvider: vi.fn(), - getProviderName: (slug: string) => slug, - getProviderLogo: () => '', hasAnyConnection: false, }); - const { container } = render(); + render(); - expect(container).toBeEmptyDOMElement(); + expect( + screen.getByRole('link', { name: /Connect an integration/i }), + ).toBeInTheDocument(); + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); }); }); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx index 1349d22c7b..f2c544c239 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx @@ -1,12 +1,31 @@ 'use client'; +import Image from 'next/image'; import Link from 'next/link'; import { useParams } from 'next/navigation'; -import { Button, Skeleton } from '@trycompai/design-system'; -import { Renew } from '@trycompai/design-system/icons'; +import { + Button, + Select, + SelectContent, + SelectItem, + SelectTrigger, + Separator, + Skeleton, +} from '@trycompai/design-system'; +import { InProgress, Renew } from '@trycompai/design-system/icons'; import { usePermissions } from '@/hooks/use-permissions'; import { useDeviceSync } from '../hooks/useDeviceSync'; +const NO_SYNC_VALUE = '__no_sync__'; + +/** + * Picks which connected integration supplies device inventory for this org — + * the device-sync counterpart of the People tab's sync source selects + * (TwoFactorSourceSelector / people sync). Always visible for users with + * integration:update: shows a "Connect an integration" slot when the org has + * no device-sync-capable connection, and lists broken (errored) connections + * as disabled options marked "Reconnect". + */ export function DeviceSyncProviderSelector() { const { orgId } = useParams<{ orgId: string }>(); const { hasPermission } = usePermissions(); @@ -21,9 +40,6 @@ export function DeviceSyncProviderSelector() { availableProviders, syncDevices, setSyncProvider, - getProviderName, - getProviderLogo, - hasAnyConnection, } = useDeviceSync({ organizationId: orgId, enabled: canManageDeviceSync }); if (!canManageDeviceSync) { @@ -34,115 +50,161 @@ export function DeviceSyncProviderSelector() { return ; } - if (!hasAnyConnection) { - // No active connection — but if one exists in an error state (e.g. expired - // OAuth), say so instead of hiding device sync entirely, so the user knows - // a reconnect brings it back. - const erroredProviders = availableProviders.filter( - (p) => p.connectionStatus === 'error', - ); - if (erroredProviders.length === 0) { - return null; - } - const names = erroredProviders.map((p) => p.name).join(', '); + const connectedProviders = availableProviders.filter((p) => p.connected); + // Broken connections (e.g. expired OAuth) — shown as disabled options so the + // user knows device sync exists and a reconnect brings it back. + const erroredProviders = availableProviders.filter( + (p) => !p.connected && p.connectionStatus === 'error', + ); + const selected = connectedProviders.find((p) => p.slug === selectedProvider); + + // Empty slot instead of nothing: the labeled placeholder shows exactly what + // this setting is and how to unlock it (mirrors TwoFactorSourceSelector). + if (connectedProviders.length === 0 && erroredProviders.length === 0) { return ( -
-
- Device sync is unavailable — the {names} connection - {erroredProviders.length > 1 ? 's need' : ' needs'} to be reconnected. -
+
+ Device sync - Go to Integrations + Connect an integration + →
); } - const connectedProviders = availableProviders.filter((p) => p.connected); - - const handleSync = async () => { - if (!selectedProvider) return; - await syncDevices(selectedProvider); + const handleValueChange = (value: string | null) => { + if (!value) return; + if (value === NO_SYNC_VALUE) { + void setSyncProvider(null); + return; + } + void syncDevices(value); }; - const handleProviderChange = (e: React.ChangeEvent) => { - void setSyncProvider(e.target.value || null); + const handleSyncNow = async () => { + if (!selected) return; + await syncDevices(selected.slug); }; return ( -
-
- {selectedProvider ? ( - <> - -
-
- {getProviderName(selectedProvider)} +
+
+ Device sync + {/* Uncontrolled on purpose — mirrors the (working) people-sync select. */} + - + {connectedProviders.map((p) => ( - + +
+ {p.logoUrl && ( + + )} + {p.name} + {selectedProvider === p.slug && ( + Active + )} +
+ ))} - - ) : null} - - {selectedProvider && ( - - )} + {erroredProviders.map((p) => ( + +
+ {p.logoUrl && ( + + )} + {p.name} + + Reconnect + +
+ + ))} + + +
+ Don't auto-sync + {!selected && ( + Active + )} +
+ + +
+ + {selected && ( + + )}
); } From 4fa759818ac26cb1d1d4d2001283bb95e46bf247 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 9 Jul 2026 14:34:38 -0400 Subject: [PATCH 3/9] fix(devices): address cubic review on device-sync connection status - connectionStatus fallback now decides by the LATEST connection row: reconnects create new rows, so a newer 'disconnected' row must not be shadowed by a stale older 'error' row (no more false reconnect hints after a deliberate disconnect) - closed select trigger shows 'Needs reconnection' instead of the misleading 'Not syncing' when the only connection(s) are broken - 'Don't auto-sync' Active marker keyed on the saved provider slug, not the resolved connected provider, so a broken chosen provider no longer mislabels auto-sync as disabled Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc --- ...ync-available-providers.controller.spec.ts | 34 +++++++++++++++++-- .../controllers/sync.controller.ts | 12 ++++--- .../DeviceSyncProviderSelector.test.tsx | 7 +++- .../components/DeviceSyncProviderSelector.tsx | 11 +++++- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts b/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts index 6782e6322d..1b6e7391b6 100644 --- a/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts @@ -127,10 +127,10 @@ describe('SyncController - getAvailableSyncProviders connection status', () => { expect(mockConnectionFindFirst).toHaveBeenCalledTimes(1); }); - it('reports connectionStatus error (not connected) when the only connection is broken', async () => { + it('reports connectionStatus error (not connected) when the latest connection is broken', async () => { mockConnectionFindFirst.mockImplementation( - (args: { where: { status: string } }) => - args.where.status === 'error' ? { id: 'icn_broken' } : null, + (args: { where: { status?: string } }) => + args.where.status === 'active' ? null : { status: 'error' }, ); const result = await controller.getAvailableSyncProviders(orgId, 'device'); @@ -145,6 +145,34 @@ describe('SyncController - getAvailableSyncProviders connection status', () => { ]); }); + it('does not resurface a stale error when the latest connection is disconnected', async () => { + // Reconnects create new rows, so an old 'error' row can coexist with a + // newer 'disconnected' one. The latest row (disconnected) must win. + mockConnectionFindFirst.mockImplementation( + (args: { where: { status?: string } }) => + args.where.status === 'active' ? null : { status: 'disconnected' }, + ); + + const result = await controller.getAvailableSyncProviders(orgId, 'device'); + + expect(result.providers).toEqual([ + expect.objectContaining({ + slug: 'intune', + connected: false, + connectionStatus: null, + connectionId: null, + }), + ]); + // The fallback must pick the LATEST row regardless of status (no status + // filter), ordered by recency — not hunt for any old 'error' row. + const fallbackArgs = mockConnectionFindFirst.mock.calls[1][0] as { + where: { status?: string }; + orderBy: { updatedAt: string }; + }; + expect(fallbackArgs.where.status).toBeUndefined(); + expect(fallbackArgs.orderBy).toEqual({ updatedAt: 'desc' }); + }); + it('reports connectionStatus null when the org has no connection for the provider', async () => { mockConnectionFindFirst.mockResolvedValue(null); diff --git a/apps/api/src/integration-platform/controllers/sync.controller.ts b/apps/api/src/integration-platform/controllers/sync.controller.ts index 4412e6b64a..9da0d3bdb0 100644 --- a/apps/api/src/integration-platform/controllers/sync.controller.ts +++ b/apps/api/src/integration-platform/controllers/sync.controller.ts @@ -1794,16 +1794,18 @@ export class SyncController { }); // No active connection: surface a broken (errored) one so the UI can // prompt a reconnect instead of silently hiding sync for the provider. - // Deliberately ignores 'disconnected' — the user chose to disconnect. - const erroredConnection = connection + // Multiple rows can exist per (org, provider) — reconnects create new + // rows — so the LATEST row decides: a newer 'disconnected' row means + // the user chose to disconnect, and a stale older 'error' row must + // not resurface a reconnect hint. + const latestConnection = connection ? null : await db.integrationConnection.findFirst({ where: { organizationId, - status: 'error', provider: { slug: m.id }, }, - select: { id: true }, + select: { status: true }, orderBy: { updatedAt: 'desc' }, }); return { @@ -1813,7 +1815,7 @@ export class SyncController { connected: !!connection, connectionStatus: connection ? ('active' as const) - : erroredConnection + : latestConnection?.status === 'error' ? ('error' as const) : null, connectionId: connection?.id ?? null, diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx index e95615ed0a..cf5d608b76 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx @@ -116,6 +116,10 @@ describe('DeviceSyncProviderSelector — RBAC gating', () => { expect( screen.getByRole('option', { name: /Kandji/i }), ).toBeInTheDocument(); + // The saved provider is still the user's choice — "Don't auto-sync" must + // NOT be marked Active just because the choice can't be resolved to a + // connected provider. + expect(screen.queryByText('Active')).not.toBeInTheDocument(); // No Sync now button without a connected selected provider. expect( screen.queryByRole('button', { name: /Sync now/i }), @@ -180,8 +184,9 @@ describe('DeviceSyncProviderSelector — connection states', () => { render(); // The select renders (not the connect slot): the org HAS a connection, - // it just needs a reconnect. + // it just needs a reconnect — and the closed trigger says so. const trigger = screen.getByRole('combobox', { name: /Sync devices from/i }); + expect(screen.getByText('Needs reconnection')).toBeInTheDocument(); await user.click(trigger); const intuneOption = await screen.findByRole('option', { name: /Intune/i }); expect(intuneOption).toHaveAttribute('aria-disabled', 'true'); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx index f2c544c239..9215220a65 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx @@ -115,6 +115,12 @@ export function DeviceSyncProviderSelector() { )} {selected.name}
+ ) : connectedProviders.length === 0 && erroredProviders.length > 0 ? ( + // The only connection(s) are broken — say so on the closed + // trigger instead of the misleading "Not syncing". + + Needs reconnection + ) : ( Not syncing )} @@ -184,7 +190,10 @@ export function DeviceSyncProviderSelector() {
Don't auto-sync - {!selected && ( + {/* Keyed on the saved slug, not the resolved connected + provider: a saved provider whose connection broke is still + the user's choice — auto-sync was never disabled. */} + {!selectedProvider && ( Active )}
From d59f0a0425c2ee79cbda53631552096211bb792e Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 9 Jul 2026 16:51:48 -0400 Subject: [PATCH 4/9] fix(devices): surface broken saved sync source on the closed trigger The 'Needs reconnection' trigger state only fired when NO provider was connected. If the saved sync source's connection broke while another provider stayed connected, the trigger fell back to 'Not syncing' even though the daily sync was failing. Key the trigger off the saved provider's errored state as well. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc --- .../DeviceSyncProviderSelector.test.tsx | 25 +++++++++++++++++++ .../components/DeviceSyncProviderSelector.tsx | 14 ++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx index cf5d608b76..754e9ae8a5 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx @@ -197,6 +197,31 @@ describe('DeviceSyncProviderSelector — connection states', () => { ).not.toBeInTheDocument(); }); + it('shows Needs reconnection when the SAVED provider is errored even if another provider is connected', () => { + mockHook({ + selectedProvider: 'intune', // the chosen sync source — its connection broke + availableProviders: [ + { ...provider, slug: 'kandji', name: 'Kandji' }, // still connected + { + ...provider, + slug: 'intune', + name: 'Intune', + connected: false, + connectionStatus: 'error', + connectionId: null, + }, + ], + hasAnyConnection: true, + }); + + render(); + + // The daily sync is failing — the closed trigger must say so, not the + // bland "Not syncing". + expect(screen.getByText('Needs reconnection')).toBeInTheDocument(); + expect(screen.queryByText('Not syncing')).not.toBeInTheDocument(); + }); + it('falls back to the connect slot when the API omits connectionStatus (older API response)', () => { mockHook({ selectedProvider: null, diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx index 9215220a65..7fa356c606 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx @@ -57,6 +57,12 @@ export function DeviceSyncProviderSelector() { (p) => !p.connected && p.connectionStatus === 'error', ); const selected = connectedProviders.find((p) => p.slug === selectedProvider); + // The saved sync source's own connection is broken — the daily sync is + // failing, which the closed trigger must surface even when other providers + // are still connected. + const selectedIsErrored = erroredProviders.some( + (p) => p.slug === selectedProvider, + ); // Empty slot instead of nothing: the labeled placeholder shows exactly what // this setting is and how to unlock it (mirrors TwoFactorSourceSelector). @@ -115,9 +121,11 @@ export function DeviceSyncProviderSelector() { )} {selected.name}
- ) : connectedProviders.length === 0 && erroredProviders.length > 0 ? ( - // The only connection(s) are broken — say so on the closed - // trigger instead of the misleading "Not syncing". + ) : selectedIsErrored || + (connectedProviders.length === 0 && erroredProviders.length > 0) ? ( + // The saved provider's connection is broken, or the only + // connection(s) are — say so on the closed trigger instead of + // the misleading "Not syncing". Needs reconnection From c45fa5f02cb3094e9e516d7f92d08f3d5aeaa80b Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Thu, 9 Jul 2026 22:21:37 -0400 Subject: [PATCH 5/9] feat(devices): universal source-reported compliance + last-seen for device sync (#3383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(devices): universal source-reported compliance for device sync Device sync's universal shape gains optional provider-reported fields: - isCompliant (overall verdict, e.g. Intune complianceState) - checks[] ({id,label,passed}, provider's own vocabulary, capped at 50) - lastSeenAt (provider's last-contact timestamp) Stored in a new nullable Device.sourceCompliance JSON column, fully separate from the agent's measured compliance columns. The devices UI renders what the source reported — verdict badge with provider attribution, provider-named check badges, honest online/offline dot from the provider timestamp — and keeps 'Not tracked' + dashes when a provider reports nothing. Agent and Fleet rendering paths untouched. All fields optional: existing device-sync definitions (Kandji, Intune, JumpCloud) keep validating unchanged; providers adopt per-field via their DB-row definitions with no deploy. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkPDc * fix(devices): address cubic review on source-reported compliance - accept timezone offsets in lastSeenAt (a +02:00 timestamp must not drop the whole device from the sync) - clamp future lastSeenAt to now so provider clock skew can't pin an imported device 'Online' for days - details panel: provider attribution uses the accessible tooltip pattern (matching stale/not-tracked) instead of a title attribute - details header: online/offline shown for imported devices too, consistent with the list - drop 'red' from a test name that didn't assert color Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../generic-device-sync.service.spec.ts | 117 ++++++++++++++++++ .../services/generic-device-sync.service.ts | 29 ++++- .../DeviceAgentDevicesList.test.tsx | 83 ++++++++++++- .../devices/components/DeviceDetails.tsx | 75 ++++++++++- .../devices/components/DeviceListCells.tsx | 56 +++++++-- .../people/devices/lib/device-source.ts | 26 +++- .../[orgId]/people/devices/lib/devices-csv.ts | 16 ++- .../[orgId]/people/devices/types/index.ts | 22 ++++ .../src/app/api/people/agent-devices/route.ts | 12 +- .../migration.sql | 5 + packages/db/prisma/schema/device.prisma | 6 + .../src/dsl/__tests__/interpreter.test.ts | 59 +++++++++ .../integration-platform/src/dsl/types.ts | 30 +++++ 13 files changed, 511 insertions(+), 25 deletions(-) create mode 100644 packages/db/prisma/migrations/20260710015503_add_device_source_compliance/migration.sql diff --git a/apps/api/src/integration-platform/services/generic-device-sync.service.spec.ts b/apps/api/src/integration-platform/services/generic-device-sync.service.spec.ts index 6aff923564..5272463c04 100644 --- a/apps/api/src/integration-platform/services/generic-device-sync.service.spec.ts +++ b/apps/api/src/integration-platform/services/generic-device-sync.service.spec.ts @@ -466,4 +466,121 @@ describe('GenericDeviceSyncService', () => { expect(result.removed).toBe(0); }); }); + // ======================================================================== + // Source-reported compliance (sourceCompliance) + provider last-seen + // ======================================================================== + + describe('Source-reported compliance', () => { + it('persists the provider verdict, checks, and lastSeenAt on create', async () => { + const lastSeenAt = '2026-07-09T18:00:00.000Z'; + await service.processDevices({ + organizationId: ORG_ID, + connectionId: CONN_ID, + devices: [ + baseDevice({ + isCompliant: true, + checks: [ + { id: 'disk_encryption', label: 'Disk Encryption', passed: true }, + { id: 'firewall', label: 'Firewall', passed: false }, + ], + lastSeenAt, + }), + ], + }); + + expect(mockDeviceCreate).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + sourceCompliance: { + isCompliant: true, + checks: [ + { id: 'disk_encryption', label: 'Disk Encryption', passed: true }, + { id: 'firewall', label: 'Firewall', passed: false }, + ], + }, + lastCheckIn: new Date(lastSeenAt), + }), + }), + ); + }); + + it('persists checks without a verdict (provider reports checks only)', async () => { + await service.processDevices({ + organizationId: ORG_ID, + connectionId: CONN_ID, + devices: [ + baseDevice({ + checks: [{ id: 'os_up_to_date', label: 'OS up to date', passed: true }], + }), + ], + }); + + const data = mockDeviceCreate.mock.calls[0][0].data; + expect(data.sourceCompliance).toEqual({ + checks: [{ id: 'os_up_to_date', label: 'OS up to date', passed: true }], + }); + expect(data.sourceCompliance.isCompliant).toBeUndefined(); + }); + + it('writes JsonNull when the provider reports no compliance (clears stale values on update)', async () => { + // Existing integration device — update path. + mockDeviceFindFirst.mockResolvedValue({ id: 'dev_1', source: 'integration' }); + + await service.processDevices({ + organizationId: ORG_ID, + connectionId: CONN_ID, + devices: [baseDevice()], + }); + + expect(mockDeviceUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + sourceCompliance: Prisma.JsonNull, + }), + }), + ); + }); + + it('treats an explicit false verdict as reported (not as absent)', async () => { + await service.processDevices({ + organizationId: ORG_ID, + connectionId: CONN_ID, + devices: [baseDevice({ isCompliant: false })], + }); + + const data = mockDeviceCreate.mock.calls[0][0].data; + expect(data.sourceCompliance).toEqual({ isCompliant: false }); + }); + + it('clamps a future lastSeenAt to now (provider clock skew must not pin the device Online)', async () => { + const future = new Date(Date.now() + 3 * 24 * 60 * 60 * 1000).toISOString(); + const before = Date.now(); + await service.processDevices({ + organizationId: ORG_ID, + connectionId: CONN_ID, + devices: [baseDevice({ lastSeenAt: future })], + }); + const after = Date.now(); + + const data = mockDeviceCreate.mock.calls[0][0].data; + const ts = (data.lastCheckIn as Date).getTime(); + expect(ts).toBeGreaterThanOrEqual(before); + expect(ts).toBeLessThanOrEqual(after); + }); + + it('falls back to sync time for lastCheckIn when lastSeenAt is absent', async () => { + const before = Date.now(); + await service.processDevices({ + organizationId: ORG_ID, + connectionId: CONN_ID, + devices: [baseDevice()], + }); + const after = Date.now(); + + const data = mockDeviceCreate.mock.calls[0][0].data; + const ts = (data.lastCheckIn as Date).getTime(); + expect(ts).toBeGreaterThanOrEqual(before); + expect(ts).toBeLessThanOrEqual(after); + }); + }); }); diff --git a/apps/api/src/integration-platform/services/generic-device-sync.service.ts b/apps/api/src/integration-platform/services/generic-device-sync.service.ts index 370fc4f71e..a216f3de34 100644 --- a/apps/api/src/integration-platform/services/generic-device-sync.service.ts +++ b/apps/api/src/integration-platform/services/generic-device-sync.service.ts @@ -160,16 +160,43 @@ export class GenericDeviceSyncService { }); } + // Compliance as reported by the source, if any. Written on EVERY sync + // (JsonNull when absent) so stale values are cleared if the provider + // stops reporting — never freeze an outdated verdict on the row. + const hasSourceCompliance = + device.isCompliant !== undefined || + (device.checks !== undefined && device.checks.length > 0); + const sourceCompliance: Prisma.InputJsonValue | typeof Prisma.JsonNull = + hasSourceCompliance + ? { + ...(device.isCompliant !== undefined + ? { isCompliant: device.isCompliant } + : {}), + ...(device.checks !== undefined && device.checks.length > 0 + ? { checks: device.checks } + : {}), + } + : Prisma.JsonNull; + const updateData = { name: device.name, hostname: device.hostname ?? device.name, platform: device.platform, osVersion: device.osVersion ?? 'Unknown', hardwareModel: device.hardwareModel, - lastCheckIn: new Date(), + // Prefer the provider's own last-contact timestamp (validated ISO + // string) so "Last seen" reflects the device, not our sync cadence. + // Clamped to now: a future timestamp (provider clock skew / bad + // mapping) would otherwise pin the device "Online" for days. + lastCheckIn: device.lastSeenAt + ? new Date( + Math.min(new Date(device.lastSeenAt).getTime(), Date.now()), + ) + : new Date(), source: 'integration' as const, integrationConnectionId: connectionId, externalDeviceId: device.externalId, + sourceCompliance, // Backfill the serial on updates too, so an externalId-matched row // becomes serial-linkable once the provider reports one. When the // device has no serial, Prisma omits `undefined` and leaves it as-is. diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx index 704fe6ef39..08b133a7ab 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx @@ -227,11 +227,84 @@ describe('DeviceAgentDevicesList — integration-imported devices', () => { ).toBeInTheDocument(); }); - it('does not present an imported device as Online or Offline', () => { - render(); - // Imported devices get no live-status dot at all (a spacer, no title). - expect(screen.queryByTitle('Online')).not.toBeInTheDocument(); - expect(screen.queryByTitle('Offline')).not.toBeInTheDocument(); + it('presents an imported device as Online when the provider saw it recently', () => { + // lastCheckIn carries the PROVIDER's last-contact timestamp for imported + // devices (device sync lastSeenAt), so the live dot applies honestly. + render( + , + ); + expect(screen.getByTitle('Online')).toBeInTheDocument(); + }); + + it('presents an imported device as Offline when the provider has not seen it recently', () => { + const threeDaysAgo = new Date(Date.now() - 3 * 24 * 60 * 60 * 1000).toISOString(); + render( + , + ); + expect(screen.getByTitle('Offline')).toBeInTheDocument(); + }); + + it('shows the SOURCE-reported verdict and named checks when the provider reports them', () => { + render( + , + ); + // Verdict shown instead of "Not tracked", with provider attribution. + expect(screen.getByText('Yes')).toBeInTheDocument(); + expect(screen.queryByText('Not tracked')).not.toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /Where does this compliance status come from\?/i }), + ).toBeInTheDocument(); + // Provider-named check badges, pass and fail. + expect(screen.getByText('Disk Encryption')).toBeInTheDocument(); + expect(screen.getByText('Firewall')).toBeInTheDocument(); + }); + + it('shows "No" when the source reports non-compliant', () => { + render( + , + ); + expect(screen.getByText('No')).toBeInTheDocument(); + expect(screen.queryByText('Not tracked')).not.toBeInTheDocument(); + }); + + it('keeps "Not tracked" when the source reports checks but no verdict', () => { + render( + , + ); + // Checks render, but with no overall verdict the compliance column stays honest. + expect(screen.getByText('OS up to date')).toBeInTheDocument(); + expect(screen.getByText('Not tracked')).toBeInTheDocument(); }); it('renders a source filter only when more than one source is present', () => { diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx index 536229aaa2..a52874cde2 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx @@ -22,6 +22,9 @@ import { CHECK_FIELDS, PLATFORM_LABELS, isDeviceOnline, + sourceChecks, + sourceReportedTooltipCopy, + sourceVerdict, staleLabel, staleTooltipCopy, } from '../lib/device-source'; @@ -30,7 +33,37 @@ import { RevokeAgentAccessDialog } from './RevokeAgentAccessDialog'; function DeviceComplianceBadge({ device }: { device: DeviceWithChecks }) { if (device.source === 'integration') { - return ; + // Show the SOURCE's own verdict when it reports one (attributed via the + // same tooltip pattern as the stale/not-tracked badges, so the provenance + // is reachable by keyboard/touch); otherwise untracked, as before. + const verdict = sourceVerdict(device); + if (verdict === undefined) { + return ; + } + return ( +
+ + {verdict ? 'Compliant' : 'Non-Compliant'} + + + + + + + + {sourceReportedTooltipCopy(device)} + + + +
+ ); } if (device.complianceStatus === 'stale') { return ( @@ -82,8 +115,11 @@ export const DeviceDetails = ({ device, onClose }: DeviceDetailsProps) => {
- {/* Live online/offline status only applies to agent devices. */} - {device.source === 'device_agent' && ( + {/* Live status: agent devices report directly; imported devices + carry the provider's last-contact timestamp (lastSeenAt), so + the same rule applies — and stays consistent with the list. */} + {(device.source === 'device_agent' || + device.source === 'integration') && ( { {device.name} - {device.source === 'device_agent' && ( + {(device.source === 'device_agent' || + device.source === 'integration') && ( {isDeviceOnline(device.lastCheckIn) ? 'Online' : 'Offline'} @@ -191,7 +228,35 @@ export const DeviceDetails = ({ device, onClose }: DeviceDetailsProps) => { - {CHECK_FIELDS.map(({ key, dbKey, label }) => { + {/* Imported device with SOURCE-reported checks: show those (provider + vocabulary) instead of CompAI's fixed agent checks. */} + {device.source === 'integration' && sourceChecks(device).length > 0 && + sourceChecks(device).map((check) => ( + + + + {check.label} + + + + + Reported by {device.integrationProvider?.name ?? 'the integration'} + + + + + {check.passed ? 'Pass' : 'Fail'} + + + + + — + + + + ))} + {!(device.source === 'integration' && sourceChecks(device).length > 0) && + CHECK_FIELDS.map(({ key, dbKey, label }) => { const isIntegration = device.source === 'integration'; const isFleetUnsupported = device.source === 'fleet' && key !== 'diskEncryptionEnabled'; diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx index 74db9d1380..9e744dbcac 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx @@ -30,7 +30,10 @@ import { isComplianceTracked, isDeviceOnline, notTrackedTooltipCopy, + sourceChecks, sourceLabel, + sourceReportedTooltipCopy, + sourceVerdict, staleLabel, staleTooltipCopy, } from '../lib/device-source'; @@ -108,11 +111,26 @@ export function NotTrackedBadge({ device }: { device: DeviceWithChecks }) { } export function CompliantBadge({ device }: { device: DeviceWithChecks }) { - // Integration-imported devices are inventory records, not compliance records — - // CompAI never ran security checks on them, so showing "No" (red) would be a - // false negative. Present them as untracked instead. + // Integration-imported devices: CompAI never ran security checks on them, so + // a red "No" from OUR checks would be a false negative. But when the SOURCE + // reports its own verdict (e.g. Intune complianceState), show that — clearly + // attributed to the provider. No verdict → untracked, as before. if (!isComplianceTracked(device)) { - return ; + const verdict = sourceVerdict(device); + if (verdict === undefined) { + return ; + } + return ( +
+ + {verdict ? 'Yes' : 'No'} + + +
+ ); } if (device.complianceStatus === 'stale') { @@ -133,6 +151,26 @@ export function CompliantBadge({ device }: { device: DeviceWithChecks }) { } export function CheckBadges({ device }: { device: DeviceWithChecks }) { + // Imported devices: render whatever checks the SOURCE reported, in the + // provider's own naming. Nothing reported → placeholder dashes, as before. + if (!isComplianceTracked(device)) { + const checks = sourceChecks(device); + if (checks.length > 0) { + return ( +
+ {checks.map((check) => ( + + {check.label} + + ))} +
+ ); + } + } if (!isComplianceTracked(device) || device.complianceStatus === 'stale') { const reason = !isComplianceTracked(device) ? 'not collected for imported devices' @@ -174,14 +212,16 @@ export function DeviceTableRow({ onRequestRemove, }: DeviceTableRowProps) { const isAgent = device.source === 'device_agent'; - const showOnline = isAgent && isDeviceOnline(device.lastCheckIn); + // Imported devices carry the PROVIDER's last-contact timestamp in + // lastCheckIn (see device sync lastSeenAt), so the same online rule applies + // honestly to them too. Fleet rows keep the spacer. + const showsLiveDot = isAgent || device.source === 'integration'; + const showOnline = showsLiveDot && isDeviceOnline(device.lastCheckIn); return ( onSelect(device)} style={{ cursor: 'pointer' }}>
- {/* Live status only applies to agent devices; imported/fleet rows get a - spacer so they aren't visually labeled as an offline agent. */} - {isAgent ? ( + {showsLiveDot ? ( { - // Integration-imported devices are inventory-only; agent + Fleet carry real - // compliance. Use the shared helper so the CSV matches the rest of the UI. + // Integration-imported devices are inventory-only for OUR checks; agent + + // Fleet carry measured compliance. When the source integration reports its + // own verdict, export it with explicit attribution. const tracked = isComplianceTracked(d); - const status = tracked ? d.complianceStatus : 'not_tracked'; + const verdict = sourceVerdict(d); + const status = tracked + ? d.complianceStatus + : verdict === undefined + ? 'not_tracked' + : verdict + ? 'compliant (source-reported)' + : 'non_compliant (source-reported)'; const check = (value: boolean) => (tracked ? yesNo(value) : 'n/a'); return [ escapeCell(d.name), diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/types/index.ts b/apps/app/src/app/(app)/[orgId]/people/devices/types/index.ts index 3ebb9834f8..4ea5648d4c 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/types/index.ts +++ b/apps/app/src/app/(app)/[orgId]/people/devices/types/index.ts @@ -11,6 +11,23 @@ export type CheckDetailEntry = { export type CheckDetails = Record; +/** A compliance check as reported by the source integration (provider naming). */ +export type SourceComplianceCheck = { + id: string; + label: string; + passed: boolean; +}; + +/** + * Compliance reported by the SOURCE integration for imported devices. Both + * fields optional — providers report what they know. null/absent = the source + * reports no compliance (UI shows "Not tracked"). + */ +export type SourceCompliance = { + isCompliant?: boolean; + checks?: SourceComplianceCheck[]; +}; + export interface DeviceWithChecks { id: string; name: string; @@ -46,6 +63,11 @@ export interface DeviceWithChecks { name: string; logoUrl?: string; }; + /** + * Set only when `source === 'integration'` and the provider reports + * compliance: the source's verdict and/or its own named checks. + */ + sourceCompliance?: SourceCompliance | null; /** Derived on the server; 'stale' = no check-in for >= 7 days. */ complianceStatus: DeviceComplianceStatus; /** Whole days since last check-in, or null when never synced. */ diff --git a/apps/app/src/app/api/people/agent-devices/route.ts b/apps/app/src/app/api/people/agent-devices/route.ts index c44fa85ae4..8e5d3aa872 100644 --- a/apps/app/src/app/api/people/agent-devices/route.ts +++ b/apps/app/src/app/api/people/agent-devices/route.ts @@ -5,7 +5,11 @@ import { daysSinceCheckIn, getDeviceComplianceStatus, } from '@trycompai/utils/devices'; -import type { CheckDetails, DeviceWithChecks } from '@/app/(app)/[orgId]/people/devices/types'; +import type { + CheckDetails, + DeviceWithChecks, + SourceCompliance, +} from '@/app/(app)/[orgId]/people/devices/types'; /** Maps the DB `DeviceSource` enum to the frontend source discriminant. */ function mapSource(source: string): DeviceWithChecks['source'] { @@ -126,6 +130,12 @@ export async function GET(req: Request) { }, source, ...(provider ? { integrationProvider: provider } : {}), + // Source-reported compliance for imported devices (null when the + // provider reports none). The stored JSON was validated against + // SyncDeviceSchema at sync time. + ...(source === 'integration' + ? { sourceCompliance: (device.sourceCompliance as SourceCompliance) ?? null } + : {}), complianceStatus, daysSinceLastCheckIn: daysSinceCheckIn(device.lastCheckIn), hasActiveAgentSession: diff --git a/packages/db/prisma/migrations/20260710015503_add_device_source_compliance/migration.sql b/packages/db/prisma/migrations/20260710015503_add_device_source_compliance/migration.sql new file mode 100644 index 0000000000..98c740e1d6 --- /dev/null +++ b/packages/db/prisma/migrations/20260710015503_add_device_source_compliance/migration.sql @@ -0,0 +1,5 @@ +-- AlterTable +ALTER TABLE "Device" ADD COLUMN "sourceCompliance" JSONB; + +-- AlterTable +ALTER TABLE "FrameworkEditorFrameworkFamily" ALTER COLUMN "updatedAt" SET DEFAULT CURRENT_TIMESTAMP; diff --git a/packages/db/prisma/schema/device.prisma b/packages/db/prisma/schema/device.prisma index 8c1b93fe7a..76655d8a4f 100644 --- a/packages/db/prisma/schema/device.prisma +++ b/packages/db/prisma/schema/device.prisma @@ -33,6 +33,12 @@ model Device { integrationConnectionId String? externalDeviceId String? + // Compliance as reported by the SOURCE integration (device sync), kept + // separate from the agent's measured columns above so provenance never + // blurs. Shape: { isCompliant?: boolean, checks?: [{id,label,passed}] }. + // NULL = the source reports no compliance (renders "Not tracked"). + sourceCompliance Json? + @@unique([serialNumber, organizationId]) // Prevents duplicate integration-sourced devices for the same connection and // also serves the device-sync fallback lookup (externalDeviceId + connection). diff --git a/packages/integration-platform/src/dsl/__tests__/interpreter.test.ts b/packages/integration-platform/src/dsl/__tests__/interpreter.test.ts index aed2a4fb34..44fce68346 100644 --- a/packages/integration-platform/src/dsl/__tests__/interpreter.test.ts +++ b/packages/integration-platform/src/dsl/__tests__/interpreter.test.ts @@ -1129,6 +1129,65 @@ describe('interpretDeclarativeDeviceSync', () => { expect(devices[0]!.name).toBe('Good'); }); + it('keeps source-reported compliance fields (verdict, named checks, lastSeenAt)', async () => { + const runner = interpretDeclarativeDeviceSync({ + definition: deviceDef(`scope.devices = [ + { + name: 'PC', platform: 'windows', userEmail: 'a@x.com', status: 'active', + externalId: 'ext-1', + isCompliant: true, + checks: [ + { id: 'disk_encryption', label: 'Disk Encryption', passed: true }, + { id: 'firewall', label: 'Firewall', passed: false }, + ], + lastSeenAt: '2026-07-09T18:00:00.000Z', + }, + ];`), + }); + + const devices = await runner.run(createMockContext()); + + expect(devices).toHaveLength(1); + expect(devices[0]).toMatchObject({ + isCompliant: true, + checks: [ + { id: 'disk_encryption', label: 'Disk Encryption', passed: true }, + { id: 'firewall', label: 'Firewall', passed: false }, + ], + lastSeenAt: '2026-07-09T18:00:00.000Z', + }); + }); + + it('accepts a lastSeenAt with a timezone offset (providers do not always report UTC)', async () => { + const runner = interpretDeclarativeDeviceSync({ + definition: deviceDef(`scope.devices = [ + { name: 'PC', platform: 'windows', userEmail: 'a@x.com', status: 'active', externalId: 'e1', lastSeenAt: '2026-07-09T18:00:00+02:00' }, + ];`), + }); + + const devices = await runner.run(createMockContext()); + + expect(devices).toHaveLength(1); + expect(devices[0]!.lastSeenAt).toBe('2026-07-09T18:00:00+02:00'); + }); + + it('drops a device with an invalid lastSeenAt or an oversized checks list, keeps valid ones', async () => { + const runner = interpretDeclarativeDeviceSync({ + definition: deviceDef(` + const tooMany = Array.from({ length: 51 }, (_, i) => ({ id: 'c' + i, label: 'C' + i, passed: true })); + scope.devices = [ + { name: 'Good', platform: 'macos', userEmail: 'a@x.com', status: 'active', externalId: 'e1' }, + { name: 'Bad date', platform: 'macos', userEmail: 'b@x.com', status: 'active', externalId: 'e2', lastSeenAt: 'yesterday' }, + { name: 'Too many checks', platform: 'macos', userEmail: 'c@x.com', status: 'active', externalId: 'e3', checks: tooMany }, + ];`), + }); + + const devices = await runner.run(createMockContext()); + + expect(devices).toHaveLength(1); + expect(devices[0]!.name).toBe('Good'); + }); + it('reads from a custom devicesPath', async () => { const runner = interpretDeclarativeDeviceSync({ definition: deviceDef( diff --git a/packages/integration-platform/src/dsl/types.ts b/packages/integration-platform/src/dsl/types.ts index b3d6a52168..884a934247 100644 --- a/packages/integration-platform/src/dsl/types.ts +++ b/packages/integration-platform/src/dsl/types.ts @@ -318,6 +318,21 @@ export type SyncDefinition = z.infer; // Sync Device Schema (for dynamic device sync) // ============================================================================ +/** + * A single security/compliance check as reported by the SOURCE (the MDM / + * provider), in the provider's own vocabulary. Universal on purpose: every + * provider reports a different subset (or none), so nothing here is assumed. + */ +export const SyncDeviceCheckSchema = z.object({ + /** Stable slug from the provider, e.g. 'disk_encryption', 'firewall'. */ + id: z.string().min(1).max(64), + /** Display name in the provider's own wording, e.g. 'BitLocker'. */ + label: z.string().min(1).max(120), + passed: z.boolean(), +}); + +export type SyncDeviceCheck = z.infer; + export const SyncDeviceSchema = z.object({ name: z.string(), platform: z.enum(['macos', 'windows', 'linux']), @@ -328,6 +343,21 @@ export const SyncDeviceSchema = z.object({ userEmail: z.string(), status: z.enum(['active', 'inactive']), externalId: z.string().optional(), + /** + * Provider-reported compliance — all optional because providers differ in + * what (if anything) they report. Omitted fields render as "Not tracked" in + * the UI; only what the source actually knows is shown. + */ + isCompliant: z.boolean().optional(), + /** Capped so a buggy definition can't stuff megabytes into device rows. */ + checks: z.array(SyncDeviceCheckSchema).max(50).optional(), + /** + * When the device last contacted the PROVIDER (e.g. Intune lastSyncDateTime), + * ISO 8601. Feeds the device list's "Last seen" column and online indicator. + * offset:true — providers commonly return timezone offsets (+02:00), and a + * rejected timestamp would drop the whole device from the sync. + */ + lastSeenAt: z.string().datetime({ offset: true }).optional(), metadata: z.record(z.string(), z.unknown()).optional(), }); From f94f02ac255fb56b0cbe354dc80e6aa26e72194a Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Thu, 9 Jul 2026 23:01:26 -0400 Subject: [PATCH 6/9] feat(devices): count imported devices in the compliance chart (#3384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(devices): count imported devices in the compliance chart The Device Compliance donut previously excluded integration-imported devices entirely, so its total contradicted the device table right below it. Now every device counts: imported devices use the SOURCE's verdict (sourceCompliance.isCompliant) for Compliant/Non-Compliant, and imported devices with no verdict land in a gray 'Not Tracked' segment instead of being fabricated into a verdict. The Not Tracked legend entry only appears when it exists, so agent/Fleet-only orgs see no change. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc * fix(devices): compliance chart colors — dead legacy tokens rendered black CHART_COLORS referenced hsl(var(--chart-positive)) etc. from the legacy @trycompai/ui stylesheet, which the app no longer loads — so all pie segments fell back to SVG-default black. Use the design-system tokens the app actually defines: --primary (brand green) for compliant, --destructive for non-compliant, --muted-foreground for not tracked. Dark mode adapts automatically. Note: vendors/backup-overview charts reference the same dead tokens (same black rendering) — left for a follow-up, out of scope here. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../components/DeviceComplianceChart.test.tsx | 75 +++++++++++++++++++ .../components/DeviceComplianceChart.tsx | 50 +++++++++---- 2 files changed, 110 insertions(+), 15 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.test.tsx index 5b5e0f059b..2e3069e764 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.test.tsx @@ -219,4 +219,79 @@ describe('DeviceComplianceChart', () => { // Array.every on empty array returns true expect(screen.getByText('Compliant (1)')).toBeInTheDocument(); }); + + it('does not show a Not Tracked legend entry when every device has a verdict', () => { + render( + , + ); + expect(screen.queryByText(/Not Tracked/)).not.toBeInTheDocument(); + }); +}); + +function makeIntegrationDevice( + overrides: Partial = {}, +): DeviceWithChecks { + return makeAgentDevice({ + source: 'integration', + integrationProvider: { slug: 'intune', name: 'Microsoft Intune' }, + // Server derives non_compliant for imported rows; the chart must use the + // source verdict (or Not Tracked), never this derived status. + isCompliant: false, + complianceStatus: 'non_compliant', + agentVersion: null, + ...overrides, + }); +} + +describe('DeviceComplianceChart — integration-imported devices', () => { + it('counts an imported device with a source verdict of compliant as Compliant', () => { + render( + , + ); + expect(screen.getByText('Compliant (1)')).toBeInTheDocument(); + expect(screen.getByText('Non-Compliant (1)')).toBeInTheDocument(); + expect(screen.queryByText(/Not Tracked/)).not.toBeInTheDocument(); + }); + + it('counts an imported device with a source verdict of non-compliant as Non-Compliant', () => { + render( + , + ); + expect(screen.getByText('Non-Compliant (1)')).toBeInTheDocument(); + }); + + it('puts an imported device with no source verdict into a Not Tracked segment', () => { + render( + , + ); + expect(screen.getByText('Compliant (1)')).toBeInTheDocument(); + expect(screen.getByText('Non-Compliant (0)')).toBeInTheDocument(); + expect(screen.getByText('Not Tracked (1)')).toBeInTheDocument(); + }); + + it('renders the chart (not the empty state) for an org with ONLY imported devices', () => { + render( + , + ); + expect(screen.queryByText(/No device data available/)).not.toBeInTheDocument(); + expect(screen.getByText('Compliant (1)')).toBeInTheDocument(); + }); }); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.tsx index 32b6a59d6a..0345049698 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.tsx @@ -16,20 +16,22 @@ interface DeviceComplianceChartProps { agentDevices: DeviceWithChecks[]; } +// Design-system tokens (full oklch values — no hsl() wrapper). The previous +// `hsl(var(--chart-positive))` tokens came from the legacy @trycompai/ui +// stylesheet the app no longer loads, so every segment silently rendered as +// SVG-default black. const CHART_COLORS = { - compliant: 'hsl(var(--chart-positive))', - nonCompliant: 'hsl(var(--chart-destructive))', + compliant: 'var(--primary)', // brand green + nonCompliant: 'var(--destructive)', // red + notTracked: 'var(--muted-foreground)', // gray — no compliance data available }; export function DeviceComplianceChart({ fleetDevices, agentDevices }: DeviceComplianceChartProps) { - // Integration-imported devices carry no compliance data, so they must not be - // counted as non-compliant here — this chart reflects compliance-tracked - // devices only (agent + Fleet). - const trackedAgentDevices = React.useMemo( - () => (agentDevices ?? []).filter((d) => d.source === 'device_agent'), - [agentDevices], - ); - const devices = [...trackedAgentDevices, ...(fleetDevices ?? [])]; + // ALL devices count — the chart total must match the table so the page never + // looks self-contradictory. Imported devices use the SOURCE's verdict when + // it reports one; imported devices with no verdict land in a gray + // "Not tracked" segment rather than being fabricated into a verdict. + const devices = [...(agentDevices ?? []), ...(fleetDevices ?? [])]; const { pieDisplayData, legendDisplayData } = React.useMemo(() => { if (devices.length === 0) { @@ -37,10 +39,19 @@ export function DeviceComplianceChart({ fleetDevices, agentDevices }: DeviceComp } let compliantCount = 0; let nonCompliantCount = 0; + let notTrackedCount = 0; - // Count device-agent devices. Stale devices (no check-in for >= 7 days) - // count as non-compliant to match the table's three-state rendering. - for (const device of trackedAgentDevices) { + for (const device of agentDevices ?? []) { + if (device.source === 'integration') { + // Source-reported verdict (e.g. Intune complianceState), if any. + const verdict = device.sourceCompliance?.isCompliant; + if (verdict === true) compliantCount++; + else if (verdict === false) nonCompliantCount++; + else notTrackedCount++; + continue; + } + // Device-agent devices. Stale devices (no check-in for >= 7 days) + // count as non-compliant to match the table's three-state rendering. if (device.complianceStatus === 'compliant') { compliantCount++; } else { @@ -69,12 +80,21 @@ export function DeviceComplianceChart({ fleetDevices, agentDevices }: DeviceComp value: nonCompliantCount, fill: CHART_COLORS.nonCompliant, }, + { + name: 'Not Tracked', + value: notTrackedCount, + fill: CHART_COLORS.notTracked, + }, ]; return { pieDisplayData: allItems.filter((item) => item.value > 0), - legendDisplayData: allItems, + // "Not Tracked" only appears in the legend when it exists — orgs with no + // verdict-less imported devices keep the familiar two-entry legend. + legendDisplayData: allItems.filter( + (item) => item.name !== 'Not Tracked' || item.value > 0, + ), }; - }, [trackedAgentDevices, fleetDevices]); + }, [agentDevices, fleetDevices]); const totalDevices = devices.length; From 425fd61d1a11e770eda5fde173437b5d3eed95f1 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Thu, 9 Jul 2026 23:15:46 -0400 Subject: [PATCH 7/9] refactor(devices): right-align device-sync control with icon-only re-sync button (#3385) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(devices): right-align device-sync control, icon-only re-sync button The sync-source select moves to the right end of its row (it's a setting, not the page subject), with the re-sync button snug beside it as an icon-only button — the select already names what's syncing, so the 'Sync now' text was redundant. aria-label + title keep it accessible; the connect-an-integration empty slot right-aligns too so the control doesn't jump sides between states. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc * refactor(devices): match device-sync control to design — inline synced-ago + primary icon button Inline row per Tofik's mockup: 'Synced Xh ago' muted text, compact provider select (label dropped — the provider name + refresh affordance identify the control), and the re-sync button in the brand primary color, icon-only. Connect-an-integration slot keeps its label. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TfDHFTwRgYxUoyyqGkHPDc --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../DeviceSyncProviderSelector.test.tsx | 26 +++++++++++ .../components/DeviceSyncProviderSelector.tsx | 45 ++++++++++++------- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx index 754e9ae8a5..fcb9b5853b 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx @@ -138,6 +138,32 @@ describe('DeviceSyncProviderSelector — RBAC gating', () => { }); }); +describe('DeviceSyncProviderSelector — last synced text', () => { + beforeEach(() => { + mockHasPermission.mockImplementation( + (resource: string, action: string) => + resource === 'integration' && action === 'update', + ); + }); + + it('shows "Synced Xh ago" next to the control when the selected provider has synced', () => { + const threeHoursAgo = new Date(Date.now() - 3 * 60 * 60 * 1000).toISOString(); + mockHook({ + availableProviders: [{ ...provider, lastSyncAt: threeHoursAgo }], + }); + + render(); + + expect(screen.getByText('Synced 3h ago')).toBeInTheDocument(); + }); + + it('shows no synced text when the provider has never synced', () => { + render(); + + expect(screen.queryByText(/^Synced /)).not.toBeInTheDocument(); + }); +}); + describe('DeviceSyncProviderSelector — connection states', () => { beforeEach(() => { mockHasPermission.mockImplementation( diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx index 7fa356c606..c40fa36fb3 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx @@ -14,6 +14,7 @@ import { } from '@trycompai/design-system'; import { InProgress, Renew } from '@trycompai/design-system/icons'; import { usePermissions } from '@/hooks/use-permissions'; +import { formatTimeAgo } from '../lib/device-source'; import { useDeviceSync } from '../hooks/useDeviceSync'; const NO_SYNC_VALUE = '__no_sync__'; @@ -66,17 +67,20 @@ export function DeviceSyncProviderSelector() { // Empty slot instead of nothing: the labeled placeholder shows exactly what // this setting is and how to unlock it (mirrors TwoFactorSourceSelector). + // Right-aligned like the populated control so the slot doesn't jump sides. if (connectedProviders.length === 0 && erroredProviders.length === 0) { return ( -
- Device sync - - Connect an integration - → - +
+
+ Device sync + + Connect an integration + → + +
); } @@ -96,9 +100,16 @@ export function DeviceSyncProviderSelector() { }; return ( -
-
- Device sync + // Right-aligned inline row: "Synced Xh ago · [provider select] · [↻]". + // The provider name + refresh affordance say what this is; the last-synced + // text gives the freshness at a glance without opening the dropdown. +
+ {selected?.lastSyncAt && ( + + Synced {formatTimeAgo(selected.lastSyncAt)} + + )} +
{/* Uncontrolled on purpose — mirrors the (working) people-sync select. */} @@ -222,9 +216,10 @@ export function DeviceSyncProviderSelector() {
{selected && ( - // Icon-only re-sync in the brand primary: the select already says - // what's syncing — aria-label + title keep it accessible. + // Icon-only re-sync, neutral outline — a lone refresh icon doesn't + // warrant primary emphasis. aria-label + title keep it accessible.