[comp] Production Deploy#3019
Open
github-actions[bot] wants to merge 21 commits into
Open
Conversation
…Integration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… schema Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two-phase device sync: imports active devices (matching by member email, serial number, or external ID) and removes disappeared devices from the connection. Follows the same pattern as GenericEmployeeSyncService. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Guard Phase 2 deletion when no devices were successfully processed (prevents false deletes)
- Handle P2002 unique constraint violation on device create with fallback to update
- Replace `include: { user: true }` with `select: { id: true }` on member lookup
- Wrap Phase 2 deleteMany in try/catch to prevent uncaught DB errors
- Add test verifying no deletions occur when all devices are skipped
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… endpoints Add device sync endpoints to the SyncController: - GET device-sync-provider: read the configured device sync provider - POST device-sync-provider: set/clear the device sync provider with validation - GET available-providers?syncType=device: filter providers by device_sync capability - POST dynamic/:providerSlug/devices: run DSL-based device sync with schema validation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Validate body.deviceSyncDefinition through SyncDefinitionSchema (applying defaults) and store it on both PUT upsert and POST create endpoints. Repository upsertBySlug and create methods updated to accept and pass through the new field. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a new run-device-sync Trigger.dev task that calls the existing device sync API endpoint for a single org+connection. The daily integration-checks-schedule orchestrator now also finds orgs with deviceSyncProvider set and triggers device sync tasks for each. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Check apiClient error before updating state in setSyncProvider - Remove incorrect 401 connection error-marking in trigger task - Track all active device identifiers before member lookup in Phase 2 - Include memberId in device update for ownership changes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses the cubic review plus critical issues found in an adversarial review of PR #2802 (device import from integrations). Make it work: - device sync ran through the EMPLOYEE interpreter (SyncEmployeeSchema), so every device was dropped and nothing ever imported. Add a standalone interpretDeclarativeDeviceSync that resolves devicesPath and validates each item with SyncDeviceSchema; the controller now uses it (redundant re-validate removed). - allow authoring device-sync integrations: add the device_sync capability and deviceSyncDefinition (+ devicesPath) to the dynamic integration schema. Make it safe: - gate Phase 2 removal behind isDirectorySource (default false), mirroring the employee sync. A non-authoritative or partial provider response can no longer hard-delete devices (which cascade-delete their Findings). Hard delete stays behind the gate with a warning to convert it to a soft removal first. - Phase 1 and the P2002 fallback no longer hijack agent/Fleet devices that share a hardware serial; devices with no serialNumber and no externalId are skipped. Cubic findings: - maxDuration was in milliseconds (~7 days); Trigger.dev expects seconds. - DeviceSyncProviderSelector controls are gated on integration:update. - the P2002 fallback update now refreshes memberId on ownership change. UX: - partial-failure syncs now surface a toast instead of failing silently. - connection.lastSyncAt is recorded so the selector shows "Last synced". Tests: device interpreter (4), service gating/source-scoping/identifier guard, maxDuration regression, and RBAC component tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the second cubic review on PR #2802: - schedule: report success based on whether every queued task batch was dispatched, instead of always returning success: true (was masking batchTrigger failures; this schedule now also dispatches device syncs). - device sync endpoint: verify the connectionId actually belongs to providerSlug, so a connection can't be driven through the wrong provider manifest/sync logic. - set-device-sync-provider: normalize blank/whitespace provider to null so an empty string is never persisted and later picked up by the scheduler. - Device: add @@unique([integrationConnectionId, externalDeviceId]) (+ migration) to prevent duplicate integration devices and back the fallback lookup; the P2002 fallback now resolves the conflict by serial OR externalDeviceId (and no longer queries with an undefined identifier). - DeviceSyncProviderSelector: show the provider picker when the saved provider is no longer connected (was leaving the UI stuck); gate the useDeviceSync hook on the integration:update permission so users without it make no API calls. - useDeviceSync: abort the sync when persisting the provider choice fails, instead of syncing with a stale/unsaved provider. Tests: externalId P2002 fallback (service), picker visibility + hook-disabled gating (component), abort-on-set-failure + no-fetch-when-disabled (hook). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- backfill serialNumber on updates so an externalId-matched device becomes serial-linkable once the provider reports one (prevents later mismatch / duplication / pruning). - setDeviceSyncProvider: accept only a non-empty string; non-string input no longer throws 500 on .trim() and normalizes to null. - schedule: success now also reports failure when any device-sync dispatch throws, not just task-check batches. - useDeviceSync: revalidate provider metadata after a sync so the selector's "Last synced" reflects the new value instead of staying stale. Note: the Phase 2 hard delete (cubic P1) is intentionally gated behind isDirectorySource (default false) so it never runs; converting it to a recoverable soft-removal is deferred per the chosen "no pruning yet" approach (see the warning comment on Phase 2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… 400 setDeviceSyncProvider now distinguishes null (explicit clear, allowed) from malformed input (non-string, or blank/whitespace), returning 400 instead of silently coercing to null and clearing the org's configured provider. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rations feat: device import from integrations
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
1 issue found across 26 files
Confidence score: 4/5
- This PR is likely safe to merge, with low-to-moderate risk driven by a single medium-severity concern rather than a functional break.
- In
apps/api/src/integration-platform/controllers/sync.controller.ts,setDeviceSyncProvideruses an inline body type ({ provider: string | null }) instead of a DTO class, which can weaken request validation consistency and Swagger schema quality. - Because the issue is structural/API-contract hygiene (not a confirmed runtime regression), the merge risk stays limited.
- Pay close attention to
apps/api/src/integration-platform/controllers/sync.controller.ts- ensuresetDeviceSyncProvidergets a proper DTO with class-validator and Swagger decorators.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/integration-platform/controllers/sync.controller.ts">
<violation number="1" location="apps/api/src/integration-platform/controllers/sync.controller.ts:1702">
P2: `setDeviceSyncProvider` uses an inline type `{ provider: string | null }` for its body instead of a proper DTO class with Swagger and class-validator decorators</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| @Post('device-sync-provider') | ||
| @ApiOperation({ summary: 'Set the device sync provider' }) | ||
| @RequirePermission('integration', 'update') | ||
| async setDeviceSyncProvider( |
Contributor
There was a problem hiding this comment.
P2: setDeviceSyncProvider uses an inline type { provider: string | null } for its body instead of a proper DTO class with Swagger and class-validator decorators
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/integration-platform/controllers/sync.controller.ts, line 1702:
<comment>`setDeviceSyncProvider` uses an inline type `{ provider: string | null }` for its body instead of a proper DTO class with Swagger and class-validator decorators</comment>
<file context>
@@ -1671,6 +1674,87 @@ export class SyncController {
+ @Post('device-sync-provider')
+ @ApiOperation({ summary: 'Set the device sync provider' })
+ @RequirePermission('integration', 'update')
+ async setDeviceSyncProvider(
+ @OrganizationId() organizationId: string,
+ @Body() body: { provider: string | null },
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.
Summary by cubic
Adds device sync from integrations across the stack: new DSL interpreter, API endpoints, scheduled tasks, UI provider selector, and DB fields to safely import and manage devices with “last synced” visibility.
New Features
@trycompai/integration-platform: addeddevice_synccapability,SyncDeviceschema,interpretDeclarativeDeviceSync, and support fordeviceSyncDefinition(with optionaldevicesPath).syncType=device, and run dynamic device sync with credential refresh, run logging, and connection validation.isDirectorySource(default false).run-device-synctask in@trigger.dev/sdkand daily orchestration to dispatch device syncs for orgs with a configured provider; fixes maxDuration (seconds) and success reporting.integration:update;useDeviceSynchook handles provider selection, sync, and “Last synced” revalidation.Migration
Device.source,integrationConnectionId,externalDeviceId(with unique index on[integrationConnectionId, externalDeviceId]),Organization.deviceSyncProvider, andDynamicIntegration.deviceSyncDefinition.device_syncand provide adeviceSyncDefinition(usedevicesPathif notdevices).Written for commit 1fd3cdf. Summary will update on new commits.