Skip to content

Clean up setup experience cancellation behavior #43437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iansltx merged 43 commits into from
Apr 14, 2026
Merged

Clean up setup experience cancellation behavior #43437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
775af8b
🤖 Clean up setup experience cancellation behavior
iansltx Apr 11, 2026
ad3112f
Tweak changes file
iansltx Apr 11, 2026
f597975
Match filter label to Figma
iansltx Apr 11, 2026
5fbe3fe
Add "during setup experience" copy when relevant on installs
iansltx Apr 11, 2026
4f35a80
🤖 Add test coverage for VPP install failure during setup experience a…
iansltx Apr 11, 2026
7641ce4
Merge branch 'main' into 🤖-34288-rev-2
iansltx Apr 12, 2026
5ef9c8d
Clean up cancellation logic
iansltx Apr 12, 2026
125699d
🤖 Catch up tests
iansltx Apr 12, 2026
17b2c1c
🤖 FIx more tests
iansltx Apr 12, 2026
662d192
Spelling consistency
iansltx Apr 13, 2026
25dbf52
Mark setup experience cancellations as from automation
iansltx Apr 13, 2026
b755699
🤖 Move activity emission for setup experience cxl to when triggering …
iansltx Apr 13, 2026
6e857d5
Remove references to previous behavior in comments since these are ne…
iansltx Apr 13, 2026
1fb3469
🤖 Add test coverage for setup experience cancel activity creation
iansltx Apr 13, 2026
23df02d
Fix incremental lint issues
iansltx Apr 13, 2026
ffdc483
Fix stale error message
iansltx Apr 13, 2026
16ab658
Tweak error message
iansltx Apr 13, 2026
efec8cf
Change another log line
iansltx Apr 13, 2026
e563b19
Tweak another error message
iansltx Apr 13, 2026
00f2c6b
Clean up changes file
iansltx Apr 13, 2026
7aa229d
Rename test
iansltx Apr 13, 2026
9bb544e
Don't emit activity until setup exp status persist succeeds to avoid …
iansltx Apr 13, 2026
3b67efb
Fall back to Fleet as actor name in new activity
iansltx Apr 13, 2026
6b31ec3
Fix JS lint
iansltx Apr 13, 2026
d3666ab
Add `from_setup_experience` field to install/script run activities
iansltx Apr 13, 2026
159194a
Tweak next upcoming activity handling for app store app installs with…
iansltx Apr 13, 2026
41b9500
Remove reference to nonexistent software display name field in activi…
iansltx Apr 13, 2026
e48bbd2
🤖 Add activity tests
iansltx Apr 13, 2026
573ee28
🤖 Fix test activity assertions
iansltx Apr 13, 2026
0d9f3d3
Revert audit-logs.md changes from setup experience commit (moving to …
iansltx Apr 13, 2026
f3c7552
Fix missing from_setup_experience expectation in integration test
iansltx Apr 13, 2026
cf68927
🤖 Move activity test to avoid including Fleet core activities from th…
iansltx Apr 13, 2026
da46cc6
🤖 Fix some more activity properties
iansltx Apr 13, 2026
612230e
🤖 Fix edge case on updating setup experience status result statuses
iansltx Apr 13, 2026
6bbf54a
🤖 Clean up SELECT -> UPDATEs to single queries
iansltx Apr 13, 2026
268c78d
🤖 Clean up dead code on terminal state check
iansltx Apr 13, 2026
ba617d4
Merge branch 'main' into 🤖-34288-rev-2
iansltx Apr 13, 2026
59ddb1b
Remove unused import
iansltx Apr 13, 2026
29650fa
Update test comment/name
iansltx Apr 13, 2026
ecb0333
Remove simulated bug test
iansltx Apr 13, 2026
e08a85e
Fix host-only activity flag added back due to merge conflict
iansltx Apr 13, 2026
f0dfef9
Modernize pointers
iansltx Apr 13, 2026
13ce6c6
Add missing "during setup experience" activity item strings
iansltx Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changes/34288-setup-experience-cancel-activity
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Added activity when setup experience is canceled due to software install failure
- Added cancel activities for each VPP app install skipped due to setup experience cancellation, and switched "failed" activity to "canceled" for package-based software installs in the same situation
- Added install failure activity when VPP installs fail due to licensing issues during setup experience
2 changes: 2 additions & 0 deletions cmd/fleet/cron.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,7 @@ func newAppleMDMWorkerSchedule(
commander *apple_mdm.MDMAppleCommander,
bootstrapPackageStore fleet.MDMBootstrapPackageStore,
vppInstaller fleet.AppleMDMVPPInstaller,
newActivityFn fleet.NewActivityFunc,
) (*schedule.Schedule, error) {
const (
name = string(fleet.CronAppleMDMWorker)
Expand All @@ -1087,6 +1088,7 @@ func newAppleMDMWorkerSchedule(
Commander: commander,
BootstrapPackageStore: bootstrapPackageStore,
VPPInstaller: vppInstaller,
NewActivityFn: newActivityFn,
}

w.Register(appleMDM)
Expand Down
2 changes: 1 addition & 1 deletion cmd/fleet/serve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1249,7 +1249,7 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
if err := cronSchedules.StartCronSchedule(func() (fleet.CronSchedule, error) {
commander := apple_mdm.NewMDMAppleCommander(mdmStorage, mdmPushService)
vppInstaller := svc.(fleet.AppleMDMVPPInstaller)
return newAppleMDMWorkerSchedule(ctx, instanceID, ds, logger, commander, bootstrapPackageStore, vppInstaller)
return newAppleMDMWorkerSchedule(ctx, instanceID, ds, logger, commander, bootstrapPackageStore, vppInstaller, svc.NewActivity)
}); err != nil {
initFatal(err, "failed to register apple_mdm_worker schedule")
}
Expand Down
6 changes: 3 additions & 3 deletions ee/server/service/devices.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,10 @@ func (svc *Service) getHostSetupExperienceStatus(ctx context.Context, host *flee
return nil, ctxerr.Wrap(ctx, err, "listing setup experience results")
}

// Mark canceled items as failed.
err = svc.failCancelledSetupExperienceInstalls(ctx, host.ID, hostUUID, host.DisplayName(), results)
// Add activities for canceled installs + setup experience run
err = svc.recordCanceledSetupExperienceSoftwareActivities(ctx, host.ID, hostUUID, host.DisplayName(), results)
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "failing cancelled setup experience installs")
return nil, ctxerr.Wrap(ctx, err, "recording cancelled setup experience installs")
}

var software []*fleet.SetupExperienceStatusResult
Expand Down
56 changes: 19 additions & 37 deletions ee/server/service/orbit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,8 @@ func (svc *Service) GetOrbitSetupExperienceStatus(ctx context.Context, orbitNode
}
}

err = svc.failCancelledSetupExperienceInstalls(ctx, host.ID, host.UUID, host.DisplayName(), res)
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "failing cancelled setup experience installs")
if err = svc.recordCanceledSetupExperienceSoftwareActivities(ctx, host.ID, host.UUID, host.DisplayName(), res); err != nil {
return nil, ctxerr.Wrap(ctx, err, "recording cancelled setup experience installs")
}

payload := &fleet.SetupExperienceStatusPayload{
Expand Down Expand Up @@ -229,7 +228,7 @@ func (svc *Service) GetOrbitSetupExperienceStatus(ctx context.Context, orbitNode
return payload, nil
}

func (svc *Service) failCancelledSetupExperienceInstalls(
func (svc *Service) recordCanceledSetupExperienceSoftwareActivities(
ctx context.Context,
hostID uint,
hostUUID string,
Expand All @@ -241,51 +240,34 @@ func (svc *Service) failCancelledSetupExperienceInstalls(
continue
}
r.Status = fleet.SetupExperienceStatusFailure
svc.logger.InfoContext(ctx, "marking setup experience software as failed due to cancellation", "host_uuid", hostUUID, "software_name", r.Name)
svc.logger.InfoContext(ctx, "emitting activity for canceled setup experience software", "host_uuid", hostUUID, "software_name", r.Name)
err := svc.ds.UpdateSetupExperienceStatusResult(ctx, r)
if err != nil {
return ctxerr.Wrap(ctx, err, "failing cancelled setup experience software install")
return ctxerr.Wrap(ctx, err, "marking canceled setup experience software install as failed")
}
// TODO -- support recording activity for failed VPP apps as well.
// https://github.com/fleetdm/fleet/issues/34288
if r.IsForSoftwarePackage() {
softwarePackage := ""
var source *string
installerMeta, err := svc.ds.GetSoftwareInstallerMetadataByID(ctx, *r.SoftwareInstallerID)
if err != nil && !fleet.IsNotFound(err) {
return ctxerr.Wrap(ctx, err, "getting software installer metadata for cancelled setup experience software install")
}
if installerMeta != nil {
softwarePackage = installerMeta.Name
// Get the software title to retrieve the source
if installerMeta.TitleID != nil {
title, err := svc.ds.SoftwareTitleByID(ctx, *installerMeta.TitleID, nil, fleet.TeamFilter{})
if err != nil && !fleet.IsNotFound(err) {
return ctxerr.Wrap(ctx, err, "getting software title for cancelled setup experience software install")
}
if title != nil {
source = &title.Source
}
}
}
activity := fleet.ActivityTypeInstalledSoftware{
if err := svc.NewActivity(ctx, nil, fleet.ActivityTypeCanceledInstallSoftware{
HostID: hostID,
HostDisplayName: hostDisplayName,
SoftwareTitle: r.Name,
SoftwarePackage: softwarePackage,
InstallUUID: ptr.ValOrZero(r.HostSoftwareInstallsExecutionID),
Status: "failed",
SelfService: false,
Source: source,
SoftwareTitleID: ptr.ValOrZero(r.SoftwareTitleID),
FromSetupExperience: true,
}); err != nil {
return ctxerr.Wrap(ctx, err, "creating activity for canceled setup experience software install")
}
err = svc.NewActivity(ctx, nil, activity)
if err != nil {
return ctxerr.Wrap(ctx, err, "creating activity for cancelled setup experience software install")
} else if r.IsForVPPApp() {
if err := svc.NewActivity(ctx, nil, fleet.ActivityTypeCanceledInstallAppStoreApp{
HostID: hostID,
HostDisplayName: hostDisplayName,
SoftwareTitle: r.Name,
SoftwareTitleID: ptr.ValOrZero(r.SoftwareTitleID),
FromSetupExperience: true,
}); err != nil {
return ctxerr.Wrap(ctx, err, "creating activity for canceled setup experience VPP app install")
Comment on lines 242 to +266
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't fail the poll after the cancellation has already been consumed.

Line 242 flips the row to Failure before the activity write. If NewActivity fails at Line 249 or Line 259, this returns an error to Orbit even though the DB state is already mutated, and the next poll won't retry because the row is no longer Cancelled. That permanently drops the cancellation activity. Please make the activity write best-effort here, or persist the status transition and activity atomically.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@ee/server/service/orbit.go` around lines 242 - 266, The code currently sets
r.Status = fleet.SetupExperienceStatusFailure and calls
svc.ds.UpdateSetupExperienceStatusResult, then calls svc.NewActivity and returns
an error if NewActivity fails; this can permanently drop cancellations because
the DB row is already mutated. Fix by making the activity write best-effort:
persist the status transition via svc.ds.UpdateSetupExperienceStatusResult first
(or, if supported, create an atomic store method that persists both the status
and the activity), then call svc.NewActivity for the relevant branches
(r.IsForSoftwarePackage / r.IsForVPPApp) but do not return the error to Orbit if
NewActivity fails — instead log the failure with svc.logger.ErrorContext
(include hostUUID/hostID, r.Name, and the error) and continue; alternatively
implement a transactional method on the datastore to update the status and
insert the activity together (e.g., AddSetupExperienceStatusAndActivity) and
call that instead of separate UpdateSetupExperienceStatusResult + NewActivity.

Copy link
Copy Markdown
Contributor Author

@iansltx iansltx Apr 13, 2026

Choose a reason for hiding this comment

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

Will have to accept the risk here, since we can't run service layer work in the same transaction. Hard-failing here is probably a little better than letting this through. Going to keep this open in case a human reviewer has different opinions.

}
}
continue
}

return nil
}

Expand Down
Loading
Loading