feat(devices): show integration-imported devices in People tab + Intune/JumpCloud device sync#3305
Merged
Merged
Conversation
…ne/JumpCloud device sync Devices imported from dynamic integrations now render correctly in the People tab instead of being mislabeled as failing agent devices. - agent-devices route + types: pass the real device source (agent/fleet/ integration) and the importing provider; drop integration rows that duplicate an agent serial - People > Devices table: Source column, source filter, "Not tracked" compliance for imported devices (no false-red checks), agent-only online status - Employee detail page, People compliance roll-up, compliance chart, CSV export, and people score: treat imported devices as inventory-only, not compliance-tracked, so they no longer skew compliance or suppress Fleet - tools/device-sync-definitions: version-controlled Intune + JumpCloud deviceSyncDefinition DSL, an offline mock test harness, and an apply script (the DSL otherwise lives only in the DB) Tests: devices suite, people-score helper, and 13 definition mock tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015iDU78gxNH9Wp9sex1BDLS
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
11 issues found across 19 files
Confidence score: 3/5
apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.tscurrently handles formula-trigger sanitization before final CSV escaping, which can let integration provider names re-open CSV formula injection in exports and expose users opening the file in spreadsheet tools — move the formula-trigger check to the final escaped output path before merging.tools/device-sync-definitions/test.mjsmarks async tests as passed before their assertions run, so failingexpectThrowscases can report ✓ and then crash outside the failure counter — maketest()await async callbacks (or return/collect promises) so failures are reliably caught.tools/device-sync-definitions/intune.mjsandtools/device-sync-definitions/jumpcloud.mjshave paging behaviors that can silently truncate, miss, or duplicate records (200-page cap without failure signaling, and no stable sort during pagination) — add deterministic sorting and explicit fail/log behavior when page limits are hit to avoid partial-but-successful syncs.- Device UX logic across
DeviceListCells.tsx,DeviceAgentDevicesList.tsx, andDeviceDetails.tsxcan misrepresent status (Fleet shown as untracked/online), hide active filters, and limit keyboard/screen-reader access to row/actions, which can confuse users and block remediation workflows — tighten source-gating and keep filter/accessible controls available before merging.
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/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts:65">
P2: Source exports can reintroduce CSV formula injection for integration provider names because the existing sanitizer prefixes before final CSV escaping. Move formula-trigger handling to operate on the quote-escaped/final cell value before adding this new untrusted column.
(Based on your team's feedback about quote-escaping before CSV formula prefixes.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx:218">
P3: Clickable table row is mouse-only. Add keyboard activation/focus semantics or expose the details action through a focusable control.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
…finitions - lib/device-source.ts: shared sourceLabel + isComplianceTracked (used by the table and CSV so they can't drift); isComplianceTracked now treats Fleet as tracked — only integration imports are "Not tracked" - DeviceDetails: show live online/offline status only for agent devices - DeviceAgentDevicesList: keep the source filter visible while a non-default filter is active so users can't get stuck on an empty table - DeviceListCells: aria-label on the device-actions menu trigger - intune.mjs: fail loudly if pagination exceeds the page cap instead of importing a partial fleet as success - jumpcloud.mjs: stable sort on the systems + users pagination to avoid missing/duplicating records when data shifts mid-sync - test.mjs: make the harness await async tests so throw-cases are reliably caught and counted - devices-csv.test.ts: regression test proving an integration provider name can't reopen CSV formula injection (existing escaping already neutralizes it) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015iDU78gxNH9Wp9sex1BDLS
Contributor
Author
|
@cubic-dev-ai review it |
Contributor
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
4 issues found across 20 files
Confidence score: 3/5
- In
tools/device-sync-definitions/jumpcloud.mjs, broadly catching JumpCloud binding fetch errors can turn auth/permission or persistent API failures into an apparent “successful” sync with silently missing devices; that’s the highest user-impact risk in this PR. Restrict the ignore path to expected per-user not-found cases and rethrow/log other failures before merging. - Imported-device rendering is inconsistent across
apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsxandapps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx: imported/untracked devices can appear offline (gray dot) and still show Exception data while other fields say N/A. This can mislead admins during device/compliance review, so hide the status dot (or use a spacer) for non-agent devices and suppress Exception for untracked entries. tools/device-sync-definitions/test.mjscurrently ignores pagination params in the JumpCloud mock, so tests may pass even ifskip/limitlogic regresses in production. Update the mock to slice results by query parameters so pagination behavior is actually validated before merge.
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/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts:65">
P2: Source exports can reintroduce CSV formula injection for integration provider names because the existing sanitizer prefixes before final CSV escaping. Move formula-trigger handling to operate on the quote-escaped/final cell value before adding this new untrusted column.
(Based on your team's feedback about quote-escaping before CSV formula prefixes.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…racked rendering - jumpcloud.mjs: only swallow a per-user 404 on the bindings call; rethrow auth/permission/5xx so a systemic failure can't report a "successful" sync with every device silently missing its owner - DeviceListCells: render the live-status dot only for agent devices (spacer otherwise, so imports aren't shown as offline agents); make the device name a focusable button so details are reachable by keyboard, not just mouse - DeviceDetails: suppress the Exception column for untracked checks - test.mjs: JumpCloud mock now honors skip/limit (real pagination contract); added multi-page pagination + binding 401-rethrow / 404-skip tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015iDU78gxNH9Wp9sex1BDLS
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
…ine limit Extract the shared mock harness into harness.mjs and the provider tests into intune.test.mjs / jumpcloud.test.mjs; test.mjs is now a thin runner. It imports the two suites sequentially via dynamic import because they use top-level await and stub the global fetch — static imports would interleave and clobber it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015iDU78gxNH9Wp9sex1BDLS
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
…x, 429) Match the Intune retry coverage — assert a transient failure on the first systems fetch is retried and then succeeds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015iDU78gxNH9Wp9sex1BDLS
Contributor
|
🎉 This PR is included in version 3.94.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
2 participants
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.
What & why
Devices imported from dynamic integrations were rendering in the People tab as if they were failing agent devices — mislabeled as
device_agent, shown "Compliant: No" with all four checks red, and a fake "online" status — because integrations are inventory sources that don't report compliance. This PR makes the whole device surface source-aware, and adds the first two import providers (Intune + JumpCloud).Changes
Display (People tab) — source-aware, no more false-red
agent-devicesroute +DeviceWithCheckstype: pass the real devicesource(agent/fleet/integration) and the importing provider name; drop integration rows that duplicate an agent serial.DeviceListCells.tsx.)getMemberDevice+EmployeeDevice), People-table compliance roll-up (compute-device-status-map), compliance chart, and CSV export: treat imported devices as inventory-only so they don't show false compliance, skew the chart, or suppress a richer Fleet host.frameworks-people-score.helper): imported devices no longer count as "agent installed."Device sync definitions —
tools/device-sync-definitions/deviceSyncDefinitionDSL for Intune and JumpCloud (these otherwise live only in the DB, which the catalog export strips), an offline mock test harness, anapply.mjsto push them via the internal API, and a README.Testing
Follow-ups (not in this PR)
DevicesService(GET /v1/devices). Worth fixing before broad rollout.🤖 Generated with Claude Code
Summary by cubic
Make device lists source-aware to stop false “non-compliant” states for integration-imported devices. Add Intune and JumpCloud device sync with stricter pagination/error handling, plus an offline test harness with provider suites and JumpCloud retry coverage.
Bug Fixes
source(device_agent/fleet/integration) andintegrationProvider; drops imported rows that duplicate an agent serial.not_tracked/n/afor imported devices, and neutralizes formula injection in provider names.sourceLabel/isComplianceTrackedkeep table and CSV aligned; Fleet remains tracked. The source filter stays visible when active; device name is focusable and the actions menu has anaria-label.New Features
tools/device-sync-definitions/: version-controlled Intune + JumpClouddeviceSyncDefinition,apply.mjs, and an offline mock test setup split intoharness.mjswith provider suites; sequential runner avoidsfetchstub conflicts. Intune fails if pagination exceeds a cap; JumpCloud uses stable pagination and strict error handling (rethrows auth/permission/5xx; only per-user 404s are skipped). Tests cover pagination/auth and transient retries, including JumpCloud network error/5xx/429 paths.Written for commit e50dd81. Summary will update on new commits.