Add default fleet for new Windows MDM enrollments (#41787) - #49922
Merged
Conversation
IT admins can configure the fleet that hosts enrolling through user-driven Windows MDM enrollment (Windows Autopilot, Entra join) are automatically assigned to, via the Windows MDM settings page, the mdm.windows_enrollment.default_fleet config setting, or GitOps. - New windows_enrollment_config row stores the default team; the config API surfaces it by fleet name and hydrates reads from the row so team renames and deletions never serve a stale name. Deleting the fleet clears the setting via FK. - New edited_windows_enrollment_default_fleet activity, emitted only when the value changes. - The OMA-DM session persists the device-reported SMBIOS serial on still-unlinked enrollments, and orbit enrollment reverse-links by that serial and assigns the default fleet before orbit's one-shot setup-experience init, so the default fleet's software, scripts, and profiles apply during the Autopilot ESP. The DevDetail and osquery link paths keep the same assignment as fallbacks, and the EUA-token link path now shares the same post-link bookkeeping. - Hosts are only assigned when new to Fleet in this enrollment cycle: existing hosts, including ones parked in No team, keep their fleet on re-enrollment, matching macOS ABM behavior. - GitOps defers applying the setting until teams declared in the same run are created, and fleetctl generate-gitops exports it. - Windows MDM settings page redesign per Figma: programmatic enrollment toggle, User driven enrollment section with the Entra-gated Default fleet dropdown, and a Migration section.
Member
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Member
Author
|
/agentic_review |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #49922 +/- ##
==========================================
+ Coverage 68.13% 68.24% +0.11%
==========================================
Files 3947 3949 +2
Lines 251501 252209 +708
Branches 13465 13483 +18
==========================================
+ Hits 171353 172121 +768
+ Misses 64829 64670 -159
- Partials 15319 15418 +99
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
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
Adds support for configuring a default fleet (team) for new user-driven Windows MDM enrollments (Autopilot / Entra join), ensuring newly created hosts can be auto-assigned early (including during Orbit enrollment) and that the setting round-trips through config APIs and GitOps.
Changes:
- Persist/serve the Windows enrollment default fleet via a new
windows_enrollment_configtable, validate it in config updates, and emit a new audit activity when it changes. - Improve Windows enrollment linking to support reverse-linking by SMBIOS serial (persisted on unlinked enrollments) and assign the default fleet for newly created hosts.
- Update UI/CLI/GitOps export/apply to support the new
mdm.windows_enrollment.default_fleetconfiguration and display the new activity type.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| changes/41787-windows-enrollment-default-fleet | Changes entry (diff excluded by policy). |
| server/service/testing_utils_test.go | Adds safe default mock behavior for new datastore getter in service tests. |
| server/service/osquery_utils/queries.go | Assign default fleet during Windows MDM enrollment linking; adds helper + grace window logic. |
| server/service/osquery_utils/queries_test.go | Adds unit coverage for default-fleet assignment logic and updates existing tests. |
| server/service/orbit.go | Routes Windows EUA token linking through shared post-link bookkeeping; adds reverse-link-by-serial path. |
| server/service/microsoft_mdm.go | Persists SMBIOS serial on unlinked enrollments to enable reverse-linking in Orbit enroll. |
| server/service/mdm_test.go | Tests serial persistence behavior on the DevDetail linking branch. |
| server/service/appconfig.go | Hydrates config response from windows_enrollment_config; validates/persists default fleet and emits activity. |
| server/service/appconfig_test.go | Adds tests covering config modify behavior for Windows enrollment default fleet. |
| server/mock/datastore_mock.go | Extends mock datastore with Windows enrollment default fleet + serial reverse-link methods. |
| server/fleet/microsoft_mdm.go | Adds HardwareSerial to MDMWindowsEnrolledDevice. |
| server/fleet/hosts.go | Adds CreatedAt to HostLite for “new host” determination. |
| server/fleet/datastore.go | Adds datastore interface methods for serial persistence/reverse-link and default fleet config. |
| server/fleet/app.go | Adds mdm.windows_enrollment config type (WindowsEnrollment). |
| server/fleet/activities.go | Adds new activity details type for “edited Windows enrollment default fleet”. |
| server/datastore/mysql/schema.sql | Updates schema for new table and new enrollment column/index. |
| server/datastore/mysql/migrations/tables/20260724191920_AddWindowsEnrollmentDefaultFleet.go | Migration to create windows_enrollment_config and add hardware_serial to mdm_windows_enrollments. |
| server/datastore/mysql/migrations/tables/20260724191920_AddWindowsEnrollmentDefaultFleet_test.go | Migration test coverage for new table + column behavior. |
| server/datastore/mysql/microsoft_mdm.go | Implements serial persistence + reverse-link query; implements get/set for default Windows enrollment team. |
| server/datastore/mysql/microsoft_mdm_test.go | Adds MySQL datastore tests for new serial and default team behaviors. |
| server/datastore/mysql/hosts.go | Includes host created_at in HostLite select columns. |
| frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx | Renders the new Windows enrollment default fleet activity text. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/WindowsMdmPage.tsx | Redesigns Windows MDM settings UI, adds default-fleet dropdown + enrollment toggle, wires to config API. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/WindowsMdmPage.tests.tsx | Adds/updates UI tests for new controls and config API payloads. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/__styles.scss | Adds layout styling for new Windows MDM page sections. |
| frontend/interfaces/config.ts | Adds windows_enrollment typing under MDM config. |
| frontend/interfaces/activity.ts | Adds new activity enum + filter label for Windows enrollment default fleet. |
| cmd/fleetctl/fleetctl/testing_utils/testing_utils.go | Adds stateful mock behavior for get/set Windows enrollment default team during gitops tests. |
| cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.yml | Updates premium gitops fixture to include windows_enrollment.default_fleet. |
| cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings.yaml | Updates expected gitops output to include Windows enrollment setting. |
| cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings-insecure.yaml | Updates expected insecure gitops output to include Windows enrollment setting. |
| cmd/fleetctl/fleetctl/testdata/generateGitops/appConfig.json | Updates app config fixture to include Windows enrollment setting. |
| cmd/fleetctl/fleetctl/gitops.go | Defers applying default fleet until teams are created; applies it after team processing. |
| cmd/fleetctl/fleetctl/gitops_test.go | Adds test coverage for deferred Windows enrollment default fleet behavior. |
| cmd/fleetctl/fleetctl/generate_gitops.go | Exports windows_enrollment in generated GitOps output for premium. |
Files excluded by content exclusion policy (1)
- changes/41787-windows-enrollment-default-fleet
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require review |
Contributor
WalkthroughAdds Premium Windows MDM default-fleet configuration across the server, datastore, API, admin UI, and GitOps. GitOps defers application when the referenced fleet is created in the same run. Windows enrollment linking stores hardware serials and supports reverse linking. Newly linked hosts can receive the configured default fleet and pending MDM profiles. Activity tracking, team deletion handling, and tests cover configuration, persistence, GitOps, UI, and enrollment flows. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
server/service/osquery_utils/queries.go (1)
3206-3251: 🧹 Nitpick | 🔵 TrivialGrace-window heuristic and best-effort transfer logic look correct.
The asymmetric 5-minute grace window correctly distinguishes "new" vs "pre-existing" hosts without penalizing slow post-enrollment fleetd installs, and the no-team/has-team/timestamp checks are well covered by
TestMaybeAssignWindowsEnrollmentDefaultFleet.One thing worth confirming operationally:
BulkSetPendingMDMHostProfilesmarks Windows profiles pending but reconciliation itself runs on a cron rather than inline (per the datastore comment), so the newly-assigned fleet's profiles/software reach the device only after the next reconcile tick rather than immediately. Given the PR's goal of having profiles ready "during Autopilot ESP," it's worth confirming that cron cadence is short enough to reliably beat typical ESP timeouts.🤖 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/osquery_utils/queries.go` around lines 3206 - 3251, Confirm operationally that the cron-driven reconciliation triggered after maybeAssignWindowsEnrollmentDefaultFleet and BulkSetPendingMDMHostProfiles runs frequently enough for Windows profiles and software to reach devices during Autopilot ESP, before typical ESP timeouts; adjust the reconciliation cadence or flow only if this timing is insufficient.cmd/fleetctl/fleetctl/gitops_test.go (1)
4658-4691: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeed an existing default fleet before testing "clears" and "no-op" scenarios.
Both the "empty value is accepted and clears" and "omitted key is a no-op" cases start from
windowsEnrollmentDefaultTeamID == nil, so neither actually proves the behavior it's named for: clearing an existing default, or preserving an existing default when the key is omitted. Consider pre-populatingwindowsEnrollmentDefaultTeamID(e.g. to a known team) before running these two scenarios so the assertions verify the transition, not just the terminal state.Also applies to: 4728-4746
🤖 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 `@cmd/fleetctl/fleetctl/gitops_test.go` around lines 4658 - 4691, Update the setup for the “empty value is accepted and clears” and “omitted key is a no-op” cases in the gitops test table to seed an existing windowsEnrollmentDefaultTeamID for the target team before each scenario runs. Keep the clear case asserting the value becomes nil, and change the omitted-key assertion to verify the pre-existing default remains unchanged while preserving the existing success-output checks.
🤖 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 `@cmd/fleetctl/fleetctl/gitops.go`:
- Around line 562-576: Update the GitOps global-configuration handling around
checkWindowsEnrollmentAssignment so the configured default fleet is tracked even
when it already exists, not only when windowsEnrollmentFleetMissing is true.
Reuse the existing ABM/VPP fleet-protection and deletion-validation pattern so
--delete-other-fleets cannot remove the configured default fleet when omitted
from team YAML. Add a regression case covering an existing default fleet omitted
from team configuration.
In `@server/service/appconfig.go`:
- Around line 2258-2272: The validateWindowsEnrollment method must distinguish
null from an explicit clear: return without changing persisted settings when
WindowsEnrollment is Set but invalid, matching the EnableDiskEncryption pattern.
Only process valid WindowsEnrollment objects, allowing a valid object with an
empty DefaultFleet to clear the default fleet.
In `@server/service/orbit.go`:
- Around line 332-359: Add service-layer Orbit unit tests covering the
reverse-link branch around
MDMWindowsGetUnlinkedEnrolledDeviceWithHardwareSerial: verify NotFound is
ignored, non-NotFound lookup errors are logged, and a successful lookup whose
LinkWindowsHostMDMEnrollment call fails is handled and logged. Reuse the
existing Orbit test setup and mocks, and ensure the Windows-enabled, configured,
non-empty hardware-serial conditions activate the branch.
---
Nitpick comments:
In `@cmd/fleetctl/fleetctl/gitops_test.go`:
- Around line 4658-4691: Update the setup for the “empty value is accepted and
clears” and “omitted key is a no-op” cases in the gitops test table to seed an
existing windowsEnrollmentDefaultTeamID for the target team before each scenario
runs. Keep the clear case asserting the value becomes nil, and change the
omitted-key assertion to verify the pre-existing default remains unchanged while
preserving the existing success-output checks.
In `@server/service/osquery_utils/queries.go`:
- Around line 3206-3251: Confirm operationally that the cron-driven
reconciliation triggered after maybeAssignWindowsEnrollmentDefaultFleet and
BulkSetPendingMDMHostProfiles runs frequently enough for Windows profiles and
software to reach devices during Autopilot ESP, before typical ESP timeouts;
adjust the reconciliation cadence or flow only if this timing is insufficient.
🪄 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 Plus
Run ID: 4008c47d-bb61-460b-b72b-57641ba7a201
📒 Files selected for processing (35)
changes/41787-windows-enrollment-default-fleetcmd/fleetctl/fleetctl/generate_gitops.gocmd/fleetctl/fleetctl/gitops.gocmd/fleetctl/fleetctl/gitops_test.gocmd/fleetctl/fleetctl/testdata/generateGitops/appConfig.jsoncmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings-insecure.yamlcmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings.yamlcmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.ymlcmd/fleetctl/fleetctl/testing_utils/testing_utils.gofrontend/interfaces/activity.tsfrontend/interfaces/config.tsfrontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/WindowsMdmPage.tests.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/WindowsMdmPage.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/__styles.scssserver/datastore/mysql/hosts.goserver/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/microsoft_mdm_test.goserver/datastore/mysql/migrations/tables/20260724191920_AddWindowsEnrollmentDefaultFleet.goserver/datastore/mysql/migrations/tables/20260724191920_AddWindowsEnrollmentDefaultFleet_test.goserver/datastore/mysql/schema.sqlserver/fleet/activities.goserver/fleet/app.goserver/fleet/datastore.goserver/fleet/hosts.goserver/fleet/microsoft_mdm.goserver/mock/datastore_mock.goserver/service/appconfig.goserver/service/appconfig_test.goserver/service/mdm_test.goserver/service/microsoft_mdm.goserver/service/orbit.goserver/service/osquery_utils/queries.goserver/service/osquery_utils/queries_test.goserver/service/testing_utils_test.go
Contributor
There was a problem hiding this comment.
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 35 out of 36 changed files in this pull request and generated 3 comments.
Files excluded by content exclusion policy (1)
- changes/41787-windows-enrollment-default-fleet
Contributor
There was a problem hiding this comment.
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 43 out of 44 changed files in this pull request and generated 1 comment.
Files excluded by content exclusion policy (1)
- changes/41787-windows-enrollment-default-fleet
Contributor
There was a problem hiding this comment.
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 44 out of 45 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/41787-windows-enrollment-default-fleet
…enrollment-default-fleet # Conflicts: # frontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tsx
Contributor
There was a problem hiding this comment.
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 54 out of 55 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/41787-windows-enrollment-default-fleet
Suppressed comments (2)
server/service/orbit.go:361
- Reverse-linking by SMBIOS serial should likely skip well-known placeholder/non-unique serials (same as the DevDetail linking path) to avoid accidental cross-linking and unnecessary DB lookups.
ee/server/service/teams.go:1013 - Clearing the Windows enrollment default fleet before attempting to delete the team can leave a persistent side effect if ds.DeleteTeam fails (team remains, but default fleet is cleared). Consider moving the SetWindowsEnrollmentDefaultFleet(nil) call to after a successful team deletion (or relying on the FK + cache invalidation after delete).
clearedWindowsEnrollmentDefaultFleet := winDefaultFleetID != nil && *winDefaultFleetID == teamID
if clearedWindowsEnrollmentDefaultFleet {
if err := svc.ds.SetWindowsEnrollmentDefaultFleet(ctx, nil); err != nil {
return ctxerr.Wrap(ctx, err, "clear windows enrollment default fleet")
}
Contributor
There was a problem hiding this comment.
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 54 out of 55 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/41787-windows-enrollment-default-fleet
Suppressed comments (2)
server/service/orbit.go:361
- The reverse-link-by-serial branch can link enrollments for devices that report a placeholder/non-unique SMBIOS serial (common on VMs), which can incorrectly associate another device’s unlinked enrollment to this host. Since the codebase already defines
fleet.IsPlaceholderHardwareSerial(and uses it in the DevDetail path), this branch should also skip placeholder serials.
ee/server/service/teams.go:1009 - This clears the Windows enrollment default fleet before attempting
DeleteTeam. IfDeleteTeamfails, the team still exists but the default fleet setting has been cleared (inconsistent state). Sincewindows_enrollment_config.team_idis an FK withON DELETE SET NULL, the setting will be cleared automatically on successful delete; this pre-clear is unnecessary and risky.
if clearedWindowsEnrollmentDefaultFleet {
if err := svc.ds.SetWindowsEnrollmentDefaultFleet(ctx, nil); err != nil {
return ctxerr.Wrap(ctx, err, "clear windows enrollment default fleet")
}
}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/fleetctl/fleetctl/gitops_test.go (1)
4773-4803: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winTest the persisted default-fleet state transitions.
These cases start with
defaultTeamIDset tonil. The empty value case cannot prove that GitOps clears an existing default fleet. The omitted-key case cannot prove that GitOps preserves one. Add a literalwindows_enrollment: nullcase because generated GitOps emits that value. Seed a default fleet, count setter calls, and assert that empty clears while omitted andnullretain the existing fleet.Also applies to: 4848-4864
🤖 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 `@cmd/fleetctl/fleetctl/gitops_test.go` around lines 4773 - 4803, Extend the Windows enrollment GitOps tests around the “empty value is accepted and clears” and “omitted key is a no-op” cases to seed an existing default fleet and track default-fleet setter calls. Add a literal null-value case matching generated GitOps output. Assert that an empty value clears the persisted fleet, while an omitted key and null preserve it, with setter-call counts confirming each transition.
🤖 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.
Outside diff comments:
In `@cmd/fleetctl/fleetctl/gitops_test.go`:
- Around line 4773-4803: Extend the Windows enrollment GitOps tests around the
“empty value is accepted and clears” and “omitted key is a no-op” cases to seed
an existing default fleet and track default-fleet setter calls. Add a literal
null-value case matching generated GitOps output. Assert that an empty value
clears the persisted fleet, while an omitted key and null preserve it, with
setter-call counts confirming each transition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 84effa2b-cd28-4e6a-9bdc-e3550e521bfe
📒 Files selected for processing (15)
changes/41787-windows-enrollment-default-fleetcmd/fleetctl/fleetctl/generate_gitops.gocmd/fleetctl/fleetctl/generate_gitops_test.gocmd/fleetctl/fleetctl/gitops.gocmd/fleetctl/fleetctl/gitops_test.gocmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerYaml.ymlcmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigYaml.ymlcmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.ymlcmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigEmpty.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigSet.ymlcmd/fleetctl/fleetctl/testing_utils/testing_utils.go
🚧 Files skipped from review as they are similar to previous changes (5)
- changes/41787-windows-enrollment-default-fleet
- cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.yml
- cmd/fleetctl/fleetctl/testing_utils/testing_utils.go
- cmd/fleetctl/fleetctl/generate_gitops.go
- cmd/fleetctl/fleetctl/gitops.go
| } | ||
| clearedWindowsEnrollmentDefaultFleet := winDefaultFleetID != nil && *winDefaultFleetID == teamID | ||
| if clearedWindowsEnrollmentDefaultFleet { | ||
| if err := svc.ds.SetWindowsEnrollmentDefaultFleet(ctx, nil); err != nil { |
Contributor
There was a problem hiding this comment.
In think this write is redundant, no? Because the FK has ON DELETE SET NULL
Member
Author
There was a problem hiding this comment.
@juan-fdz-hawa The main reason to write is for cache invalidation, since this setting is part of appconfig. And since we already did the read to see if it changed (for activity), why not do the write? In practice, we will actually rarely do the write, so it seems fine to do it if needed.
juan-fdz-hawa
left a comment
juan-fdz-hawa
left a comment
Contributor
There was a problem hiding this comment.
All done with the first pass
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/server/service/teams.go (1)
999-1012: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep the default fleet when team deletion fails.
SetWindowsEnrollmentDefaultFleet(ctx, nil)succeeds beforesvc.ds.DeleteTeam. IfDeleteTeamreturns an error, the team remains but loses its default-enrollment assignment.Make the deletion and default-fleet update atomic. Alternatively, invalidate the cache only after a successful deletion without changing persistent configuration before that result.
🤖 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 `@ee/server/service/teams.go` around lines 999 - 1012, Update the team-deletion flow around SetWindowsEnrollmentDefaultFleet and DeleteTeam so a failed DeleteTeam leaves the existing Windows enrollment default unchanged. Perform the deletion before clearing the default, or use an atomic transaction; only invalidate or clear the default-fleet assignment after successful deletion.
🤖 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.
Outside diff comments:
In `@ee/server/service/teams.go`:
- Around line 999-1012: Update the team-deletion flow around
SetWindowsEnrollmentDefaultFleet and DeleteTeam so a failed DeleteTeam leaves
the existing Windows enrollment default unchanged. Perform the deletion before
clearing the default, or use an atomic transaction; only invalidate or clear the
default-fleet assignment after successful deletion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 497b0666-7381-4875-a5d9-dbd4830d06df
📒 Files selected for processing (11)
ee/server/service/teams.gofrontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/WindowsMdmPage.tsxserver/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/microsoft_mdm_test.goserver/datastore/mysql/migrations/tables/20260803135530_AddWindowsEnrollmentDefaultFleet.goserver/datastore/mysql/schema.sqlserver/datastore/mysql/setup_experience.goserver/datastore/mysql/testing_utils_test.goserver/fleet/app.goserver/service/orbit.go
🚧 Files skipped from review as they are similar to previous changes (4)
- server/fleet/app.go
- frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx
- frontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/WindowsMdmPage.tsx
- server/service/orbit.go
Contributor
There was a problem hiding this comment.
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 55 out of 56 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/41787-windows-enrollment-default-fleet
Suppressed comments (1)
frontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tsx:501
- The new disabled-tooltip wrapper is only triggerable via hover (TooltipWrapper uses
mouseenterfor non-mobile and the wrapper element isn’t focusable), but the Select is also giventabIndex={-1}when disabled. That makes the “why is this disabled?” tooltip inaccessible to keyboard-only users and many touch users.
{isDisabled && disabledTooltipContent ? (
<TooltipWrapper
className={`${baseClass}__disabled-tooltip`}
tipContent={disabledTooltipContent}
position="top"
Member
Author
|
@juan-fdz-hawa I addressed/fixed your review comments. Ready for re-review. |
juan-fdz-hawa
approved these changes
Aug 4, 2026
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.
Demo: https://www.youtube.com/watch?v=cWxZlu9WuwA
Guide updates: https://github.com/fleetdm/fleet/pull/49603/changes
IT admins can configure the fleet that hosts enrolling through user-driven Windows MDM enrollment (Windows Autopilot, Entra join) are automatically assigned to, via the Windows MDM settings page, the mdm.windows_enrollment.default_fleet config setting, or GitOps.
Related issue: Resolves #41787
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/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.Timeouts are implemented and retries are limited to avoid infinite loops
Testing
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
Database migrations
COLLATE utf8mb4_unicode_ci).New Fleet configuration settings
fleetctl generate-gitopsSummary by CodeRabbit
New Features
Documentation