Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
Adds support for tracking when a Windows MDM enrollment is “awaiting configuration” (primarily for Autopilot/OOBE enrollments) by persisting new state on mdm_windows_enrollments.
Changes:
- Adds
awaiting_configuration+awaiting_configuration_atcolumns tomdm_windows_enrollments(migration + schema.sql). - Computes awaiting-configuration state during Windows enrollment and stores it on the enrollment row.
- Extends the Windows enrollment model and insert/upsert logic to write the new fields.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/microsoft_mdm.go | Sets awaiting-configuration flags during Windows MDM enrollment based on programmatic vs automatic flow and OOBE state. |
| server/fleet/microsoft_mdm.go | Extends MDMWindowsEnrolledDevice to include awaiting-configuration fields. |
| server/datastore/mysql/microsoft_mdm.go | Writes awaiting-configuration fields on insert/upsert for Windows enrollment rows. |
| server/datastore/mysql/schema.sql | Updates mdm_windows_enrollments schema (and migration status) to include new columns. |
| server/datastore/mysql/migrations/tables/20260406120000_AddWindowsAwaitingConfiguration.go | Adds the new columns to mdm_windows_enrollments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/datastore/mysql/migrations/tables/20260406120000_AddWindowsAwaitingConfiguration.go
Show resolved
Hide resolved
WalkthroughThis pull request adds Windows enrollment awaiting configuration tracking to the mdm_windows_enrollments table. Changes include:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/datastore/mysql/microsoft_mdm.go (1)
135-153:⚠️ Potential issue | 🟠 MajorDon’t make the new enrollment fields write-only.
Lines 135-136 add
awaiting_configurationandawaiting_configuration_atto the upsert, butMDMWindowsGetEnrolledDeviceWithDeviceIDandMDMWindowsGetEnrolledDeviceWithHostUUIDin this file still select the old column set. Any code rehydratingfleet.MDMWindowsEnrolledDevicethrough those helpers will always see zero values for the new state.🛠️ Suggested follow-up
SELECT id, mdm_device_id, mdm_hardware_id, device_state, device_type, device_name, enroll_type, enroll_user_id, enroll_proto_version, enroll_client_version, not_in_oobe, + awaiting_configuration, + awaiting_configuration_at, credentials_hash, credentials_acknowledged, created_at, updated_at, host_uuid FROM mdm_windows_enrollmentsApply the same addition to both
MDMWindowsGetEnrolledDeviceWithDeviceIDandMDMWindowsGetEnrolledDeviceWithHostUUID.Also applies to: 158-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/microsoft_mdm.go` around lines 135 - 153, The upsert adds awaiting_configuration and awaiting_configuration_at but the read helpers still select the old columns, so add these two columns to the SELECT list and scanning targets in both MDMWindowsGetEnrolledDeviceWithDeviceID and MDMWindowsGetEnrolledDeviceWithHostUUID (and any similar getters around lines 158-175) so fleet.MDMWindowsEnrolledDevice.awaiting_configuration and awaiting_configuration_at are populated; ensure the SELECT order matches the Scan destination order (or use named scans) and update any variable names used in those Scan calls to accept the new fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@server/datastore/mysql/migrations/tables/20260406120000_AddWindowsAwaitingConfiguration.go`:
- Around line 13-20: The migration currently returns early if
awaiting_configuration exists, which can skip adding awaiting_configuration_at;
update the idempotency logic around the columnExists checks so both columns are
verified individually (use columnExists(tx, "mdm_windows_enrollments",
"awaiting_configuration") and columnExists(tx, "mdm_windows_enrollments",
"awaiting_configuration_at")) and only return nil if both columns already exist,
or alternatively run separate ALTER TABLE statements to add each missing column
rather than a single early return before executing tx.Exec that adds both
columns.
---
Outside diff comments:
In `@server/datastore/mysql/microsoft_mdm.go`:
- Around line 135-153: The upsert adds awaiting_configuration and
awaiting_configuration_at but the read helpers still select the old columns, so
add these two columns to the SELECT list and scanning targets in both
MDMWindowsGetEnrolledDeviceWithDeviceID and
MDMWindowsGetEnrolledDeviceWithHostUUID (and any similar getters around lines
158-175) so fleet.MDMWindowsEnrolledDevice.awaiting_configuration and
awaiting_configuration_at are populated; ensure the SELECT order matches the
Scan destination order (or use named scans) and update any variable names used
in those Scan calls to accept the new fields.
🪄 Autofix (Beta)
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
Run ID: 9011ca67-dba2-45b6-9538-0a9b86d094b7
📒 Files selected for processing (5)
server/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/migrations/tables/20260406120000_AddWindowsAwaitingConfiguration.goserver/datastore/mysql/schema.sqlserver/fleet/microsoft_mdm.goserver/service/microsoft_mdm.go
server/datastore/mysql/migrations/tables/20260406120000_AddWindowsAwaitingConfiguration.go
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43067 +/- ##
=======================================
Coverage 66.84% 66.85%
=======================================
Files 2578 2579 +1
Lines 206869 206904 +35
Branches 9283 9283
=======================================
+ Hits 138291 138322 +31
- Misses 56006 56008 +2
- Partials 12572 12574 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
| _, err := tx.Exec(` | ||
| ALTER TABLE mdm_windows_enrollments | ||
| ADD COLUMN awaiting_configuration TINYINT(1) NOT NULL DEFAULT 0, |
There was a problem hiding this comment.
For whatever it's worth my initial thinking for this was this was almost a state based thing. I made it a tinyint to not make the windows enrollments rows too big but in Magnus's POC he did the same where essentially it was 0=not waiting, 1=initial waiting state, 2=later state which is basically once the full initialization/enrollment has completed and things have actually started
There was a problem hiding this comment.
Thanks. Updated to 3 states
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Related issue: Resolves #42841
This change is just new columns in a table. No other functional changes.
Checklist for submitter
Testing
Database migrations
Summary by CodeRabbit