Skip to content

iOS/iPadOS: More host vitals - #50496

Merged
lucasmrod merged 18 commits into
mainfrom
49984-ios-ipados-vitals
Aug 7, 2026
Merged

iOS/iPadOS: More host vitals#50496
lucasmrod merged 18 commits into
mainfrom
49984-ios-ipados-vitals

Conversation

@nulmete

@nulmete nulmete commented Aug 4, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #39281

Checklist for submitter

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.

  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually
ios_ipados_more_host_vitals.mov
vitals_deleted_in_byod.mov

Database migrations

  • Checked schema for all modified table for columns that will auto-update timestamps during migration.
  • Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
  • Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).

Summary by CodeRabbit

  • New Features
    • Added 29 iOS/iPadOS device vitals to host details and API responses.
    • Added a “View all” modal for browsing complete device vitals, including subscriptions and accessibility details.
    • Added support for cellular technology, organization information, attestation data, and device settings.
  • Privacy
    • Personal/BYOD enrollments exclude expanded device-vitals fields.
    • Sensitive vitals are removed from device URL-authenticated responses.
  • Bug Fixes
    • Stale vitals are cleared when enrollment ownership changes or hosts are deleted.

nulmete added 3 commits August 4, 2026 10:47
…50046)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Relates to #49984

This is PR 1 of 2 for #49984, split for ease of review and stacked onto
a feature branch (`49984-ios-ipados-vitals`). This PR adds the MySQL
storage (`host_mdm_apple_device_vitals`,
`host_mdm_apple_service_subscriptions`) for the 29 additional iOS/iPadOS
vitals, and expands the `DeviceInformation` MDM command's `Queries`
array to request the corresponding Apple keys. It does not yet parse or
persist any new ack data — that's PR 2 (#50047), stacked on top of this
branch.

# Checklist for submitter

- [ ] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

It will be included in the feature branch.

- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements), JS
inline code is prevented especially for url redirects, and untrusted
data interpolated into shell scripts/commands is validated against shell
metacharacters.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually (see results on PR2)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added support for collecting and storing Apple MDM device vitals and
cellular service subscription details.
* Expanded Apple device information requests to include the full set of
supported query fields.
* Added database support for retaining device vitals and subscription
data.

* **Bug Fixes**
* Host deletion now also removes associated Apple MDM vitals and
subscription records.

* **Tests**
* Added coverage for vitals updates, nullable values, subscription
synchronization, database constraints, and device information requests.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Relates to #49984

This is PR 2 of 2 for #49984, stacked on top of PR 1 (#50046) — diff
here is scoped to just the ack-parsing/wiring work on top of that PR's
storage and command changes. This PR parses the additional
`DeviceInformation` ack fields requested by PR 1's command expansion and
persists them via `SetOrUpdateHostMDMAppleDeviceVitals`. A persistence
failure is logged rather than aborting the MDM check-in, since it's a
non-critical write the next refetch will redo.

# Checklist for submitter

- [ ] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

It will be included in the feature branch.

## Testing

- [x] Added/updated automated tests

- [x] QA'd all new/changed functionality manually

Enrolled my iPhone to Fleet (dual-SIM, physical + eSIM):

<img width="1458" height="830" alt="Screenshot 2026-07-28 at 10 57
00 AM"
src="https://github.com/user-attachments/assets/1735b5e6-ac2d-4dc1-a66b-c8d8063b7c76"
/>

`host_mdm_apple_device_vitals` — populated as expected:
- `battery_level: 0.87`, `cellular_technology: 1`, all `is_*` booleans,
`last_cloud_backup_date` — parsed correctly
- `device_properties_attestation`: JSON array of **2** base64 DER certs
(leaf + "Apple Enterprise Attestation Sub CA" intermediate) — confirms
it's a cert chain, not a boolean, per the deviation noted in the parent
issue
- `mdm_options: {}` — key present, no sub-options applicable → empty
object, not `NULL` (expected `omitempty` behavior)
- `push_token: NULL` — expected; Apple only returns this for
user-channel enrollments, not device-channel
- `organization_info` / `accessibility_settings: NULL` — expected,
nothing configured/toggled
- `model_number`, `modem_firmware_version`,
`supplemental_build_version`, `bluetooth_mac`, `wifi_mac`,
`eas_device_identifier`, `itunes_store_account_hash` — all populated

`host_mdm_apple_service_subscriptions` — **2 rows** for one dual-SIM
device, confirming the multi-slot replace logic works on real hardware:
- `CTSubscriptionSlotOne` — fully populated (carrier, MCC/MNC, ICCID,
phone number, etc.)
- `CTSubscriptionSlotTwo` — only `eid`/`imei` populated, everything else
`NULL` — an inactive/unprovisioned eSIM slot reporting a sparse row,
exactly the shape the tests were built to mirror



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Apple device management now captures and stores additional iPhone and
iPad device vitals during device information refreshes.
* Supports details such as Wi‑Fi MAC address, lost mode status,
operating system information, accessibility settings, organization
details, and service subscriptions.

* **Bug Fixes**
* Missing or unexpected device information no longer interrupts refresh
processing.
* Failures saving supplementary device vitals no longer prevent other
device management actions from completing.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #49985

Exposes the the 29 iOS/iPadOS device vitals fields in `GET /hosts/:id`
and `GET /hosts/identifier/:identifier` for iOS/iPadOS hosts.

# Checklist for submitter

- [ ] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

Will be added in feature branch.

## Testing

- [x] Added/updated automated tests

- [x] QA'd all new/changed functionality manually


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Host details for iOS and iPadOS devices now include available Apple
MDM device vitals and service subscription information.
* Vitals are shown only when present, while unsupported platforms and
missing data remain omitted.
* Added support for nested accessibility, organization, MDM options, and
attestation details in host responses.

* **Bug Fixes**
* Improved handling of hosts without stored Apple MDM vitals, avoiding
errors and incomplete responses.

* **Tests**
* Added coverage for complete, partial, and missing vitals data across
host detail endpoints.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Copilot AI lite review requested due to automatic review settings August 4, 2026 14:25
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #49986

Adds a "View all" button to the Vitals card for iOS/iPadOS hosts,
opening a new modal that lists the 29 new device vitals alphabetically.

`View all` button is only shown for iOS/iPadOS hosts, and the vitals
card is capped to two full rows. Clicking `View all` shows all the
vitals, including the new 29 apple device vitals. See Figma:
https://www.figma.com/design/x5pERdbyiHYv0U86CfJCpd/-39281-iOS-iPadOS--More-host-vitals?node-id=5598-359&t=FUpSwSHydm81VMgg-0

`Service subscriptions` vitals are rendered as `Service subscription 1`,
`Service subscription 2`, and so on. Hovering over each of them shows
all the key-value pairs for that subscription.

<img width="1778" height="970" alt="Screenshot 2026-08-03 at 8 41 23 AM"
src="https://github.com/user-attachments/assets/3d212d0c-2e85-4715-8bb1-92db8520d8a2"
/>

NOTE: the Figma/spec suggested to add a `Not supported` value + tooltip
for any vital that is not supported for a particular platform/enrollment
type, but I couldn't notice any after the manual tests that were run on
this branch (my own iPhone, and Magnus/Rachel's iPad/iPhones). If during
QA we detect some fields not being supported for certain enrollments, I
can definitely add that code back.

# Checklist for submitter


- [ ] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

Will be added in the feature branch.

## Testing

- [x] Added/updated automated tests

- [x] QA'd all new/changed functionality manually

<img width="828" height="1340" alt="Screenshot 2026-08-03 at 8 45 01 AM"
src="https://github.com/user-attachments/assets/912c21dc-47fc-4989-b93f-2777fdc993b4"
/>
<img width="836" height="1164" alt="Screenshot 2026-08-03 at 8 46 05 AM"
src="https://github.com/user-attachments/assets/ecbe33b1-81c5-4fb9-9ee3-826329ad8dfa"
/>




<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added a “View all” option for iOS and iPadOS host vitals, opening a
dedicated Vitals modal.
* The Vitals modal presents a complete, alphabetically sorted vitals
list with improved formatting for booleans, percentages, dates, nested
values, and subscription entries.

* **Bug Fixes**
* Updated vitals behavior to correctly hide Location when applicable
(including ADE mode rules).
* Improved handling for unsupported/unavailable vitals by showing clear
“Not supported” messaging and tooltips.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds first-class support for iOS/iPadOS “device vitals” collected via Apple MDM DeviceInformation: expands the command’s queried keys, persists the resulting fields in new side tables, and surfaces them on the GET host endpoints (while ensuring fields are omitted when not applicable).

Changes:

  • Add new MySQL tables + datastore methods to upsert/load iOS/iPadOS device vitals and service subscriptions.
  • Expand Apple MDM DeviceInformation command queries and parse the additional fields from refetch acknowledgements.
  • Add unit/integration coverage ensuring fields are present for iOS/iPadOS and fully omitted for other platforms / missing rows.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server/service/integration_core_test.go Integration test asserting vitals JSON keys are present/omitted correctly across host platforms and partial/no-row cases.
server/service/hosts.go Loads iOS/iPadOS vitals into host details response for Apple mobile platforms.
server/service/hosts_test.go Updates host details unit tests to stub the new datastore load call.
server/service/devices_endpoint_test.go Updates devices endpoint tests to stub the new datastore load call.
server/service/apple_mdm.go Persists parsed vitals from refetch results and logs failures without aborting refetch handling.
server/service/apple_mdm_test.go Adds/updates tests to validate vitals persistence behavior during refetch flows (including write-failure tolerance).
server/service/apple_mdm_device_vitals.go New parsing helpers to extract typed optional values from QueryResponses and build MDMAppleDeviceVitals.
server/mock/datastore_mock.go Extends datastore mock with vitals set/load methods.
server/mdm/apple/commander.go Expands DeviceInformation queries list to request all needed vitals keys.
server/mdm/apple/commander_test.go Adds test verifying DeviceInformation queries array content/order.
server/fleet/mdm_apple_device_vitals.go New Fleet types for vitals, service subscriptions, and cellular technology JSON (label mapping).
server/fleet/mdm_apple_device_vitals_test.go Unit tests for MDMAppleCellularTechnology JSON marshal/unmarshal behavior.
server/fleet/hosts.go Embeds vitals response struct into fleet.Host for flattened JSON output.
server/fleet/datastore.go Adds datastore interface methods for persisting/loading device vitals.
server/datastore/mysql/schema.sql Adds schema definitions for vitals and service subscriptions tables.
server/datastore/mysql/migrations/tables/20260727191311_CreateHostMDMAppleDeviceVitals.go Migration creating the new vitals and subscriptions tables.
server/datastore/mysql/migrations/tables/20260727191311_CreateHostMDMAppleDeviceVitals_test.go Migration test validating inserts + PK behavior for new tables.
server/datastore/mysql/hosts.go Registers new tables for host deletion cleanup by host UUID.
server/datastore/mysql/hosts_test.go Extends host deletion test coverage to include the new tables.
server/datastore/mysql/apple_mdm_device_vitals.go New MySQL implementation for upserting/loading vitals and replacing service subscription rows.
server/datastore/mysql/apple_mdm_device_vitals_test.go Datastore tests for insert/update, NULL handling, subscription replace, idempotency, and load behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +42 to +44
case uint64:
i := int64(n) //nolint:gosec // Apple's documented integer fields fit comfortably in int64.
return &i
Copilot AI review requested due to automatic review settings August 4, 2026 14:32
# Conflicts:
#	server/datastore/mysql/schema.sql
Four migrations landed on main after this one was written, so
20260727191311 would have sorted ahead of them. Retimestamped to
20260804143233 and regenerated schema.sql.
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.22096% with 200 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.37%. Comparing base (3de43c8) to head (a827a23).

Files with missing lines Patch % Lines
pkg/mdm/mdmtest/apple.go 7.47% 99 Missing ⚠️
server/datastore/mysql/apple_mdm_device_vitals.go 82.14% 21 Missing and 19 partials ⚠️
...ontend/pages/hosts/details/cards/Vitals/Vitals.tsx 90.78% 13 Missing ⚠️
server/service/apple_mdm_device_vitals.go 91.60% 8 Missing and 4 partials ⚠️
cmd/osquery-perf/agent.go 0.00% 8 Missing ⚠️
.../hosts/details/HostDetailsPage/HostDetailsPage.tsx 0.00% 6 Missing ⚠️
server/datastore/mysql/apple_mdm.go 60.00% 3 Missing and 3 partials ⚠️
...s/20260807120050_CreateHostMDMAppleDeviceVitals.go 91.42% 4 Missing and 2 partials ⚠️
...s/hosts/details/modals/VitalsModal/VitalsModal.tsx 95.74% 4 Missing ⚠️
server/service/hosts.go 50.00% 2 Missing and 2 partials ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #50496      +/-   ##
==========================================
+ Coverage   68.35%   68.37%   +0.02%     
==========================================
  Files        3957     3962       +5     
  Lines      254097   254834     +737     
  Branches    13530    13404     -126     
==========================================
+ Hits       173681   174253     +572     
- Misses      64860    65003     +143     
- Partials    15556    15578      +22     
Flag Coverage Δ
backend 69.61% <72.21%> (+0.01%) ⬆️
frontend 61.78% <90.45%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

Suppressed comments (4)

server/service/hosts.go:1693

  • Loading iOS/iPadOS vitals into the HostDetail response here will also affect device-authenticated endpoints that call GetHost (e.g. getDeviceHostEndpoint). That endpoint currently scrubs a handful of iOS fields but does not scrub the new vitals (UDID, WiFi/Bluetooth MAC, push token, etc.), so this change can inadvertently expose additional sensitive identifiers to device URL auth responses. Either extend the device-endpoint scrubbing to cover these fields, or add an option to exclude vitals when serving device-authenticated requests.
		if err := svc.ds.LoadHostSoftware(ctx, host, opts.IncludeCVEScores); err != nil {
			return nil, ctxerr.Wrap(ctx, err, "load host software")
		}
	}

server/service/apple_mdm.go:6248

  • SetOrUpdateHostMDMAppleDeviceVitals is called unconditionally for every Apple refetch here. Since these tables are only loaded/exposed for iOS/iPadOS hosts, this will add an extra DB write on macOS refetches and create rows that are never read. Gate the vitals parse+persist behind fleet.IsAppleMobilePlatform(host.Platform) (similar to the device-name status update just below).
	vitals := parseMDMAppleDeviceVitals(queryResponses)
	if err := svc.ds.SetOrUpdateHostMDMAppleDeviceVitals(ctx, host.UUID, vitals); err != nil {
		svc.logger.ErrorContext(ctx, "update host mdm apple device vitals from refetch", "host_uuid", host.UUID, "err", err)
	}

frontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsx:720

  • This toggle uses the captured showVitalsModal value, so if toggleVitalsModal is called multiple times before React flushes state updates (e.g. two events in the same tick), it can end up setting the same value twice instead of toggling. Use the functional setState form to make the toggle resilient to batching.
  const toggleVitalsModal = useCallback(() => {
    setShowVitalsModal(!showVitalsModal);
  }, [showVitalsModal, setShowVitalsModal]);

server/datastore/mysql/apple_mdm_device_vitals.go:188

  • This update-then-insert-on-no-match pattern can still produce duplicate-key errors if two refetch handlers race on the first write for a host (both UPDATE 0 rows, then both try INSERT). Even if the service logs-and-continues, it will generate noisy errors and can skip the service_subscriptions replace for the losing transaction. Consider using a single INSERT ... ON DUPLICATE KEY UPDATE statement for host_mdm_apple_device_vitals to make the operation atomic under concurrency and independent of RowsAffected semantics.
	return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
		result, err := sqlx.NamedExecContext(ctx, tx, updateStmt, row)
		if err != nil {
			return ctxerr.Wrap(ctx, err, "update host mdm apple device vitals")
		}
		if affected, _ := result.RowsAffected(); affected == 0 {
			if _, err := sqlx.NamedExecContext(ctx, tx, insertStmt, row); err != nil {
				return ctxerr.Wrap(ctx, err, "insert host mdm apple device vitals")
			}
		}

Copilot AI review requested due to automatic review settings August 4, 2026 14:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

Suppressed comments (2)

server/fleet/hosts.go:483

  • Comment references loadHostMDMAppleDeviceVitalsDB, but no such function exists in the codebase. This makes it harder to trace where these fields are populated. Please update the reference to the actual datastore method used to load these fields.
	// JSON response. Only populated for iOS/iPadOS hosts, via a separate
	// query (loadHostMDMAppleDeviceVitalsDB) — every field is omitted (not
	// null) for every other platform, or when a given field wasn't returned

server/fleet/mdm_apple_device_vitals.go:211

  • Comment references loadHostMDMAppleDeviceVitalsDB, but that symbol doesn't exist. Consider pointing to the actual datastore method (LoadHostMDMAppleDeviceVitals) so future readers can find the implementation.
// Every field is tagged db:"-" because these are loaded via a separate query
// for iOS/iPadOS hosts only (see loadHostMDMAppleDeviceVitalsDB), not the
// main hosts SELECT, and csv:"-" because gocsv flattens embedded struct

…#50514)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Relates to #49984

Restricts device vitals fields added by #49984 (battery level, service
subscriptions/phone numbers, accessibility settings, etc.) to non-BYOD
iOS/iPadOS enrollments only. Personal (BYOD) hosts continue to get the 9
fields Fleet requested before #49984 (device name, capacity, OS version,
product name, lost-mode status, etc.), but not the newer ones, since
those can carry PII a personal device's owner may not want their
employer to see.

Note: Raised during review of #50046, see [this
comment](#50046 (review)).

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
  * Improved support for personally enrolled iOS and iPadOS devices.
* Personally enrolled devices now receive a streamlined
device-information request.
* Vitals for personally enrolled devices display the full list without a
“View all” limit or button.
* **Bug Fixes**
* Corrected Vitals card behavior for BYOD devices while preserving
existing behavior for other hosts.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Copilot AI review requested due to automatic review settings August 4, 2026 18:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

server/service/hosts.go:1606

  • RefetchHost now fetches host MDM info unconditionally whenever any refetch command is sent. If only the certs refetch runs (doCertsRefetch=true, doAppRefetch/doDeviceInfoRefetch=false), this adds an unnecessary datastore query. Consider only calling GetHostMDM when doAppRefetch or doDeviceInfoRefetch is true.

Comment thread server/service/devices_endpoint_test.go
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #50521

`AcknowledgeDeviceInformationWithVitals` now returns the new query keys
(accessibility settings, battery level, service subscriptions,
attestation chain, etc.) so
`host_mdm_apple_device_vitals`/`host_mdm_apple_service_subscriptions`
get representative synthetic data instead of staying empty.

# Checklist for submitter

- [ ] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

It will be included in the feature branch.


## Testing

- [ ] Added/updated automated tests
- [ ] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)

- [x] QA'd all new/changed functionality manually



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Bug Fixes

* Improved Apple device management acknowledgments by including device
vitals in device-information responses.
* Apple device records now provide more complete status details,
including battery, network connectivity, accessibility, security, and
subscription information.
* Enhanced consistency of device information across management
workflows, helping ensure administrators receive more reliable and
comprehensive device status data.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Copilot AI review requested due to automatic review settings August 5, 2026 16:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated no new comments.

Suppressed comments (2)

frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx:110

  • renderLines uses the line content as the React key, which can collide if the list contains duplicate entries (e.g., identical certificates), leading to unstable rendering and React warnings. Use an index-based (or index+content) key instead.
  return (
    <div className={`${baseClass}__lines`}>
      {lines.map((line) => (
        <div key={line}>{line}</div>
      ))}
    </div>
  );

frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx:182

  • The service subscription details are only exposed via a hover tooltip, and TooltipWrapper opens/closes on mouseenter/mouseleave (no focus/blur handling). That makes subscription details inaccessible to keyboard-only and many screen reader users. Consider switching to an accessible disclosure (e.g., inline expandable details) or updating TooltipWrapper to open on focus and close on blur, then using a focusable trigger here.
  return (
    <div className={`${baseClass}__subscriptions`}>
      {subscriptions.map((sub, i) => (
        <TooltipWrapper
          key={sub.slot}
          tipContent={renderTooltipFields(subscriptionFields(sub))}
        >
          {/* Template literal keeps this one text node, so the label stays
              queryable as a whole string. */}
          {`Subscription ${i + 1}`}
        </TooltipWrapper>
      ))}
    </div>

Copilot AI review requested due to automatic review settings August 5, 2026 16:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 37 out of 38 changed files in this pull request and generated 1 comment.

Files excluded by content exclusion policy (1)
  • changes/39281-ios-ipados-device-vitals
Suppressed comments (2)

server/service/apple_mdm_device_vitals.go:45

  • plistOptInt64 converts uint64 → int64 without an overflow guard. If a device (or malformed plist) reports an out-of-range uint64, this will wrap to a negative int64 and propagate a bogus value downstream. It's cheap to harden by rejecting values > MaxInt64.
    server/service/hosts.go:1606
  • RefetchHost now fetches host MDM info unconditionally, but it's only needed when doAppRefetch or doDeviceInfoRefetch is true. When only doCertsRefetch is requested, this adds an extra datastore call that doesn't affect behavior.

Comment thread server/service/hosts.go Outdated
@nulmete
nulmete requested review from a team as code owners August 6, 2026 14:06
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds collection of 29 iOS/iPadOS device vitals through Apple MDM DeviceInformation. The server parses and stores scalar, nested, attestation, and subscription data, then exposes it in host responses for eligible enrollments. Personal enrollments do not receive or retain these fields. The host details page adds a capped vitals card and a sortable “View all” modal. Tests cover parsing, persistence, enrollment transitions, API responses, privacy scrubbing, and UI rendering.

Possibly related PRs

  • fleetdm/fleet#50046: Adds the Apple MDM vitals models, storage, and DeviceInformation flow extended here.
  • fleetdm/fleet#50147: Introduces the iOS/iPadOS vitals modal and shared vitals-card implementation extended here.
  • fleetdm/fleet#50520: Updates synthetic Apple device-vitals acknowledgements used by the performance simulator.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.92% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the primary change: adding more iOS/iPadOS host vitals.
Description check ✅ Passed The description includes the related issue, user-visible change, validation, automated tests, manual QA, and database migration checks.
Linked Issues check ✅ Passed The implementation covers the issue objectives for iOS/iPadOS vitals, API and UI support, persistence, cleanup, BYOD exclusion, and enrollment transitions.
Out of Scope Changes check ✅ Passed The changes support the linked issue, including test simulation, persistence, privacy handling, UI display, and host cleanup; no unrelated code changes are evident.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 49984-ios-ipados-vitals

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (6)
frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx (1)

95-111: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Use a stable index-based key for the attestation lines.

key={line} breaks if two entries in device_properties_attestation are identical. React then logs a duplicate-key warning. The list is static and never reordered, so the index is a safe key.

♻️ Proposed change
-      {lines.map((line) => (
-        <div key={line}>{line}</div>
+      {lines.map((line, i) => (
+        // eslint-disable-next-line react/no-array-index-key
+        <div key={`${i}-${line}`}>{line}</div>
       ))}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx` around lines
95 - 111, Update the lines.map callback in renderLines to use the item's stable
index as the React key instead of the line value, preventing duplicate keys when
attestation entries are identical; preserve the existing rendered text and
filtering behavior.
frontend/pages/hosts/details/cards/Vitals/Vitals.tsx (1)

759-777: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Consider memoizing the vitals list.

buildHostVitals and sortHostVitals run on every render, including each ResizeObserver callback that changes columnCount. Each run allocates the full JSX row set. Wrap both in useMemo keyed on the source props to limit the work to actual data changes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/pages/hosts/details/cards/Vitals/Vitals.tsx` around lines 759 - 777,
Memoize the `buildHostVitals` and `sortHostVitals` pipeline in the component
containing this code using `useMemo`, with dependencies covering all source
inputs passed to `buildHostVitals` and excluding display-only changes such as
`columnCount`. Keep the existing slicing logic outside the memo so resize
updates still recalculate the visible cap without rebuilding the full vitals
list.
frontend/pages/hosts/details/cards/Vitals/Vitals.tests.tsx (1)

499-503: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Import the cap constants instead of duplicating them.

IOS_CARD_VITAL_ROWS and FALLBACK_COLUMN_COUNT are re-declared here. If the values change in Vitals.tsx, these copies drift and the cap assertions no longer test the production cap. Export both constants from Vitals.tsx and import them in this test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/pages/hosts/details/cards/Vitals/Vitals.tests.tsx` around lines 499
- 503, Remove the local IOS_CARD_VITAL_ROWS and FALLBACK_COLUMN_COUNT
declarations in Vitals.tests.tsx, export those constants from Vitals.tsx, and
import them into the test so CAP and its assertions always use the production
values.
server/datastore/mysql/apple_mdm_device_vitals.go (1)

179-191: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Prefer an upsert over UPDATE-then-INSERT.

The transaction runs an UPDATE, then INSERTs when zero rows are affected. Two concurrent acks for the same host that both find no row take the INSERT branch, and one fails on the host_uuid primary key. INSERT ... ON DUPLICATE KEY UPDATE removes that window, halves the round trips, and removes the dependence on clientFoundRows=true that the test at server/datastore/mysql/apple_mdm_device_vitals_test.go line 158 documents.

The same pattern applies per slot at lines 271-282; (host_uuid, slot) is the primary key there, so a single upsert works for subscriptions too.

The current failure mode is bounded, because the ack handler logs a vitals write error and continues, and the next refetch retries.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/datastore/mysql/apple_mdm_device_vitals.go` around lines 179 - 191,
Replace the UPDATE-then-INSERT logic in the transaction callback with a single
MySQL upsert using INSERT ... ON DUPLICATE KEY UPDATE for host vitals,
preserving all updated columns and existing error wrapping. Apply the same
upsert pattern in the per-slot subscription persistence flow around
replaceHostMDMAppleServiceSubscriptions, using (host_uuid, slot) as the conflict
key, while retaining subscription replacement behavior.
server/mdm/apple/commander.go (1)

463-473: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rename the BYOD list and document it.

The selector is isPersonalEnrollment, but the list is named byodDeviceInformationQueryKeys. This repository already uses "BYOD" for !InstalledFromDEP (see IOSiPadOSRefetch in server/mdm/apple/apple_mdm.go, which sets isBYODDevice := !device.InstalledFromDEP). Personal enrollment and BYOD are separate flags, so the current name can mislead. Add a doc comment to the list as well, matching the comment on deviceInformationQueryKeys.

♻️ Proposed rename
-var byodDeviceInformationQueryKeys = []string{
+// personalDeviceInformationQueryKeys are the Apple query keys requested in a
+// DeviceInformation command's <Queries> array for personal (BYOD)
+// enrollments, in request order. The expanded vitals are excluded so Fleet
+// does not collect data about a device the organization does not own.
+var personalDeviceInformationQueryKeys = []string{
 	"DeviceName",

Update the reference in DeviceInformation accordingly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/mdm/apple/commander.go` around lines 463 - 473, Rename
byodDeviceInformationQueryKeys to a name reflecting personal enrollment, update
its use in DeviceInformation, and add a doc comment matching
deviceInformationQueryKeys. Do not use “BYOD” in the renamed symbol, since
personal enrollment and BYOD represent different flags.
server/service/apple_mdm_test.go (1)

7619-7620: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Assert the full LastCloudBackupDate value.

The test asserts only the year. The payload sets 2026-07-01T00:00:00Z. Compare the whole timestamp so a timezone or parse regression fails the test.

♻️ Proposed assertion
 	require.NotNil(t, gotVitals.LastCloudBackupDate)
-	require.Equal(t, 2026, gotVitals.LastCloudBackupDate.Year())
+	require.True(t, gotVitals.LastCloudBackupDate.Equal(time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)),
+		"got %s", gotVitals.LastCloudBackupDate)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/service/apple_mdm_test.go` around lines 7619 - 7620, Update the test
assertions for gotVitals.LastCloudBackupDate to compare the complete expected
timestamp from the payload, 2026-07-01T00:00:00Z, rather than asserting only its
year; retain the non-nil check and use timezone-aware timestamp equality.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx`:
- Line 21: Update EMPTY_VITAL_VALUE in VitalsModal to reuse
DEFAULT_EMPTY_CELL_VALUE from utilities/constants, unless the product
requirement explicitly mandates “None”; ensure both vitals row types display the
same empty-value text.

In
`@server/datastore/mysql/migrations/tables/20260804143233_CreateHostMDMAppleDeviceVitals.go`:
- Line 31: Change the battery_level column definition in the
CreateHostMDMAppleDeviceVitals migration from float to double, and update the
corresponding host_mdm_apple_device_vitals.battery_level definition in the
generated schema.sql to match while preserving DEFAULT NULL.

In `@server/service/hosts.go`:
- Around line 1602-1605: Update the host MDM lookup in the refetch path around
GetHostMDM to treat fleet.NotFound as an absent host_mdm row instead of
returning an error. Initialize InstalledFromDep and IsPersonalEnrollment to
false for that case, while preserving wrapped error returns for other failures,
so the certificate and application refetch branches can continue using
verifyMDMConfiguredAndConnected.

---

Nitpick comments:
In `@frontend/pages/hosts/details/cards/Vitals/Vitals.tests.tsx`:
- Around line 499-503: Remove the local IOS_CARD_VITAL_ROWS and
FALLBACK_COLUMN_COUNT declarations in Vitals.tests.tsx, export those constants
from Vitals.tsx, and import them into the test so CAP and its assertions always
use the production values.

In `@frontend/pages/hosts/details/cards/Vitals/Vitals.tsx`:
- Around line 759-777: Memoize the `buildHostVitals` and `sortHostVitals`
pipeline in the component containing this code using `useMemo`, with
dependencies covering all source inputs passed to `buildHostVitals` and
excluding display-only changes such as `columnCount`. Keep the existing slicing
logic outside the memo so resize updates still recalculate the visible cap
without rebuilding the full vitals list.

In `@frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx`:
- Around line 95-111: Update the lines.map callback in renderLines to use the
item's stable index as the React key instead of the line value, preventing
duplicate keys when attestation entries are identical; preserve the existing
rendered text and filtering behavior.

In `@server/datastore/mysql/apple_mdm_device_vitals.go`:
- Around line 179-191: Replace the UPDATE-then-INSERT logic in the transaction
callback with a single MySQL upsert using INSERT ... ON DUPLICATE KEY UPDATE for
host vitals, preserving all updated columns and existing error wrapping. Apply
the same upsert pattern in the per-slot subscription persistence flow around
replaceHostMDMAppleServiceSubscriptions, using (host_uuid, slot) as the conflict
key, while retaining subscription replacement behavior.

In `@server/mdm/apple/commander.go`:
- Around line 463-473: Rename byodDeviceInformationQueryKeys to a name
reflecting personal enrollment, update its use in DeviceInformation, and add a
doc comment matching deviceInformationQueryKeys. Do not use “BYOD” in the
renamed symbol, since personal enrollment and BYOD represent different flags.

In `@server/service/apple_mdm_test.go`:
- Around line 7619-7620: Update the test assertions for
gotVitals.LastCloudBackupDate to compare the complete expected timestamp from
the payload, 2026-07-01T00:00:00Z, rather than asserting only its year; retain
the non-nil check and use timezone-aware timestamp equality.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9adace9c-35dd-418e-b623-1a487ada0d31

📥 Commits

Reviewing files that changed from the base of the PR and between 392f62d and 1f882aa.

⛔ Files ignored due to path filters (1)
  • cmd/osquery-perf/README.md is excluded by !**/*.md
📒 Files selected for processing (39)
  • changes/39281-ios-ipados-device-vitals
  • cmd/osquery-perf/agent.go
  • frontend/__mocks__/hostMock.ts
  • frontend/interfaces/host.ts
  • frontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsx
  • frontend/pages/hosts/details/cards/Vitals/Vitals.tests.tsx
  • frontend/pages/hosts/details/cards/Vitals/Vitals.tsx
  • frontend/pages/hosts/details/cards/Vitals/_styles.scss
  • frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tests.tsx
  • frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx
  • frontend/pages/hosts/details/modals/VitalsModal/_styles.scss
  • frontend/pages/hosts/details/modals/VitalsModal/index.ts
  • pkg/mdm/mdmtest/apple.go
  • server/datastore/mysql/apple_mdm.go
  • server/datastore/mysql/apple_mdm_device_vitals.go
  • server/datastore/mysql/apple_mdm_device_vitals_test.go
  • server/datastore/mysql/apple_mdm_test.go
  • server/datastore/mysql/hosts.go
  • server/datastore/mysql/hosts_test.go
  • server/datastore/mysql/migrations/tables/20260804143233_CreateHostMDMAppleDeviceVitals.go
  • server/datastore/mysql/migrations/tables/20260804143233_CreateHostMDMAppleDeviceVitals_test.go
  • server/datastore/mysql/schema.sql
  • server/fleet/datastore.go
  • server/fleet/hosts.go
  • server/fleet/mdm.go
  • server/fleet/mdm_apple_device_vitals.go
  • server/fleet/mdm_apple_device_vitals_test.go
  • server/mdm/apple/apple_mdm.go
  • server/mdm/apple/commander.go
  • server/mdm/apple/commander_test.go
  • server/mock/datastore_mock.go
  • server/service/apple_mdm.go
  • server/service/apple_mdm_device_vitals.go
  • server/service/apple_mdm_test.go
  • server/service/devices.go
  • server/service/devices_endpoint_test.go
  • server/service/hosts.go
  • server/service/hosts_test.go
  • server/service/integration_core_test.go

Comment thread frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx Outdated
Comment thread server/service/hosts.go
Comment on lines +1602 to +1605
hostMDM, err := svc.ds.GetHostMDM(ctx, host.ID)
if err != nil {
return ctxerr.Wrap(ctx, err, "get host MDM info")
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect GetHostMDM's not-found behavior and how other callers handle it.
fd -t f 'hosts.go' server/datastore/mysql --exec ast-grep run --pattern 'func (ds *Datastore) GetHostMDM($$$) { $$$ }' --lang go
rg -nP --type=go -C 4 '\bGetHostMDM\s*\(ctx' -g '!**/*_test.go' -g '!server/mock/**'

Repository: fleetdm/fleet

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -u

echo "Locate hosts.go files"
fd -t f '^hosts\.go$' . | sed -n '1,50p'

echo
echo "Search GetHostMDM references"
rg -n --type=go '\bGetHostMDM\s*\(' -g '!**/*_test.go' -g '!server/mock/**' . || true

echo
echo "Search GetHostMDM definition"
rg -n --type=go 'func .*GetHostMDM\b|GetHostMDM\s*\(' . || true

Repository: fleetdm/fleet

Length of output: 5820


🏁 Script executed:

#!/bin/bash
set -u

echo "server/datastore/mysql/hosts.go GetHostMDM implementation and nearby test snippets"
sed -n '5150,5205p' server/datastore/mysql/hosts.go | cat -n
sed -n '1560,1665p' server/datastore/mysql/hosts_test.go | cat -n

echo
echo "server/service/hosts.go refetch area"
sed -n '1575,1655p' server/service/hosts.go | cat -n

echo
echo "similar GetHostMDM call at server/service/hosts.go:1791"
sed -n '1770,1805p' server/service/hosts.go | cat -n

echo
echo "HostMDM model docs/interface"
sed -n '400,445p' server/fleet/datastore.go | cat -n
sed -n '1300,1345p' server/service/hosts.go | cat -n

Repository: fleetdm/fleet

Length of output: 18082


🏁 Script executed:

#!/bin/bash
set -u

echo "Locate hostMDMSelect and GetHostMDM final section"
rg -n --type=go 'hostMDMSelect|FROM host_mdm|GetHostMDM\b' server/datastore/mysql/hosts.go
sed -n '5100,5150p' server/datastore/mysql/hosts.go | cat -n
sed -n '5175,5235p' server/datastore/mysql/hosts.go | cat -n

echo
echo "Check Go/sqlx ErrorWithArg behavior"
go version
tmp="$(mktemp -d)"
cat > "$tmp/check.go" <<'EOF'
package main

import "fmt"

type ErrorWithArg struct {
	Err error
	Val interface{}
}

func NewErrorWithArg(err error, arg interface{}) *ErrorWithArg {
	return &ErrorWithArg{Err: err, Val: arg}
}

func (e *ErrorWithArg) Error() string {
	return "sqlx: missing argument in arglist"
}

func main() {
	fmt.Printf("%q\n", NewErrorWithArg(QueryErr, nil))
}
EOF
cd "$tmp"
go run check.go

Repository: fleetdm/fleet

Length of output: 6476


Handle missing host MDM data before refetch commands run.

GetHostMDM returns fleet.NotFound when there is no host_mdm row. In this refetch path, that error blocks the certificate and application refetch branches that only need the MDM connection state from verifyMDMConfiguredAndConnected. Treat NotFound as no host MDM row and default InstalledFromDep and IsPersonalEnrollment to false before running those commands.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/service/hosts.go` around lines 1602 - 1605, Update the host MDM lookup
in the refetch path around GetHostMDM to treat fleet.NotFound as an absent
host_mdm row instead of returning an error. Initialize InstalledFromDep and
IsPersonalEnrollment to false for that case, while preserving wrapped error
returns for other failures, so the certificate and application refetch branches
can continue using verifyMDMConfiguredAndConnected.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked and saw that verifyMDMConfiguredAndConnected runs immediately before this and internally does an INNER JOIN against host_mdm as part of confirming the host is "connected". That means a host_mdm row is already guaranteed to exist by the time GetHostMDM is called.

# Conflicts:
#	server/datastore/mysql/schema.sql

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tests.tsx (1)

118-132: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert boolean values by vital label.

getAllByText("True") and getAllByText("False") only prove that some boolean values render. They do not verify that App analytics and Awaiting configuration receive the correct values. Use findValueCell with those labels.

Proposed test adjustment
-    render(
+    const { container } = render(
       <VitalsModal host={host} vitalsData={host} mdm={host.mdm} onExit={noop} />
     );
 
-    expect(screen.getAllByText("True").length).toBeGreaterThan(0);
-    expect(screen.getAllByText("False").length).toBeGreaterThan(0);
+    expect(findValueCell(container, "App analytics")?.textContent).toBe("True");
+    expect(findValueCell(container, "Awaiting configuration")?.textContent).toBe(
+      "False"
+    );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tests.tsx` around
lines 118 - 132, Update the test case “renders scalar and boolean vital values”
to assert boolean values by their associated vital labels using the existing
findValueCell helper. Verify that “App analytics” renders “True” and “Awaiting
configuration” renders “False”, replacing the unscoped getAllByText assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tests.tsx`:
- Line 141: Wrap the long expectation in the VitalsModal test by placing the
findValueCell call and its arguments across multiple lines, preserving the
existing assertion and value.

---

Nitpick comments:
In `@frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tests.tsx`:
- Around line 118-132: Update the test case “renders scalar and boolean vital
values” to assert boolean values by their associated vital labels using the
existing findValueCell helper. Verify that “App analytics” renders “True” and
“Awaiting configuration” renders “False”, replacing the unscoped getAllByText
assertions.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 47a2a266-b8be-4275-9893-dfb5f3fc6a7b

📥 Commits

Reviewing files that changed from the base of the PR and between 1f882aa and d1a27b5.

📒 Files selected for processing (15)
  • frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tests.tsx
  • frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx
  • server/datastore/mysql/apple_mdm.go
  • server/datastore/mysql/apple_mdm_test.go
  • server/datastore/mysql/migrations/tables/20260804143233_CreateHostMDMAppleDeviceVitals.go
  • server/datastore/mysql/schema.sql
  • server/fleet/datastore.go
  • server/fleet/hosts.go
  • server/mdm/apple/apple_mdm.go
  • server/mock/datastore_mock.go
  • server/service/apple_mdm.go
  • server/service/apple_mdm_test.go
  • server/service/hosts.go
  • server/service/hosts_test.go
  • server/service/integration_core_test.go
🚧 Files skipped from review as they are similar to previous changes (13)
  • server/fleet/datastore.go
  • server/mock/datastore_mock.go
  • server/mdm/apple/apple_mdm.go
  • server/service/apple_mdm.go
  • server/service/hosts.go
  • server/fleet/hosts.go
  • frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tsx
  • server/datastore/mysql/migrations/tables/20260804143233_CreateHostMDMAppleDeviceVitals.go
  • server/service/integration_core_test.go
  • server/datastore/mysql/apple_mdm.go
  • server/datastore/mysql/apple_mdm_test.go
  • server/service/hosts_test.go
  • server/service/apple_mdm_test.go

Comment thread frontend/pages/hosts/details/modals/VitalsModal/VitalsModal.tests.tsx Outdated
nulmete added 2 commits August 6, 2026 13:10
…r it

PR #50690 (open, not yet merged) renames two out-of-order migrations
that shipped after the v4.90.0 cut so they sort after it:
20260721090128_AddTokenInvalidToABMTokens -> 20260806154139, and
20260721160351_AddHostMDMWindowsProfilesStatus -> 20260806154150.

Cherry-picked that rename here so our migration keeps running last,
and bumped 20260804143233_CreateHostMDMAppleDeviceVitals ->
20260806161534 accordingly. Regenerated schema.sql.
lucasmrod
lucasmrod previously approved these changes Aug 6, 2026
@nulmete

nulmete commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

Waiting for #50690 to be merged to main, and then regenerate the schema/reorder migrations again.

@lucasmrod
lucasmrod merged commit 358b8c4 into main Aug 7, 2026
61 of 63 checks passed
@lucasmrod
lucasmrod deleted the 49984-ios-ipados-vitals branch August 7, 2026 12:49
@xpkoala xpkoala mentioned this pull request Aug 26, 2026
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS/iPadOS: More host vitals

4 participants