From 775af8b38c5954a29cdab7d01df4312aa9a42c37 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Fri, 10 Apr 2026 20:06:01 -0500 Subject: [PATCH 01/41] =?UTF-8?q?=F0=9F=A4=96=20Clean=20up=20setup=20exper?= =?UTF-8?q?ience=20cancellation=20behavior?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For #34288. Zed + Opus 4.6; prompt: Implement https://github.com/fleetdm/fleet/issues/34288, taking into account feedback mentioned on the ticket. Use the PRs in the last comment on the issue as inspiration. Check subtasks for more specs. --- .../34288-setup-experience-cancel-activity | 4 + cmd/fleet/cron.go | 2 + cmd/fleet/serve.go | 2 +- ee/server/service/orbit.go | 78 +++- ee/server/service/orbit_test.go | 415 ++++++++++++++++++ ee/server/service/setup_experience.go | 12 + frontend/interfaces/activity.ts | 4 + .../GlobalActivityItem/GlobalActivityItem.tsx | 28 +- .../details/cards/Activity/ActivityConfig.tsx | 2 + .../CanceledInstallSoftwareActivityItem.tsx | 5 +- .../CanceledSetupExperienceActivityItem.tsx | 35 ++ .../index.ts | 1 + server/fleet/activities.go | 43 +- server/worker/apple_mdm.go | 16 + 14 files changed, 610 insertions(+), 37 deletions(-) create mode 100644 changes/34288-setup-experience-cancel-activity create mode 100644 ee/server/service/orbit_test.go create mode 100644 frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx create mode 100644 frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/index.ts diff --git a/changes/34288-setup-experience-cancel-activity b/changes/34288-setup-experience-cancel-activity new file mode 100644 index 00000000000..4fd6269c41c --- /dev/null +++ b/changes/34288-setup-experience-cancel-activity @@ -0,0 +1,4 @@ +* Added "Canceled activity: macOS setup experience" activity when setup experience is cancelled due to a software install failure. +* Added cancel activities for VPP app installs that are skipped when setup experience is cancelled. +* Added `from_setup_experience` flag to cancelled software install activities during setup experience. +* Added install failure activity when VPP app installs fail prior to sending the MDM command during setup experience. diff --git a/cmd/fleet/cron.go b/cmd/fleet/cron.go index c76a37c1da7..02561c3a7d5 100644 --- a/cmd/fleet/cron.go +++ b/cmd/fleet/cron.go @@ -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) @@ -1087,6 +1088,7 @@ func newAppleMDMWorkerSchedule( Commander: commander, BootstrapPackageStore: bootstrapPackageStore, VPPInstaller: vppInstaller, + NewActivityFn: newActivityFn, } w.Register(appleMDM) diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index 0f7c013b992..a59eb29399d 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -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") } diff --git a/ee/server/service/orbit.go b/ee/server/service/orbit.go index dcd09e371fc..d144b2f1bb4 100644 --- a/ee/server/service/orbit.go +++ b/ee/server/service/orbit.go @@ -236,6 +236,21 @@ func (svc *Service) failCancelledSetupExperienceInstalls( hostDisplayName string, results []*fleet.SetupExperienceStatusResult, ) error { + // Find the software item that originally failed and triggered the cancellation. + // It already has SetupExperienceStatusFailure before we modify any cancelled items. + var failedSoftwareName string + var failedSoftwareTitleID uint + for _, r := range results { + if r.Status == fleet.SetupExperienceStatusFailure && r.IsForSoftware() { + failedSoftwareName = r.Name + if r.SoftwareTitleID != nil { + failedSoftwareTitleID = *r.SoftwareTitleID + } + break + } + } + + var hasCancelledSoftware bool for _, r := range results { if r.Status != fleet.SetupExperienceStatusCancelled { continue @@ -246,46 +261,61 @@ func (svc *Service) failCancelledSetupExperienceInstalls( if err != nil { return ctxerr.Wrap(ctx, err, "failing cancelled setup experience software install") } - // TODO -- support recording activity for failed VPP apps as well. - // https://github.com/fleetdm/fleet/issues/34288 if r.IsForSoftwarePackage() { - softwarePackage := "" - var source *string + var softwareTitleID uint 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 - } - } + if installerMeta != nil && installerMeta.TitleID != nil { + softwareTitleID = *installerMeta.TitleID } - activity := fleet.ActivityTypeInstalledSoftware{ + activity := fleet.ActivityTypeCanceledInstallSoftware{ HostID: hostID, HostDisplayName: hostDisplayName, SoftwareTitle: r.Name, - SoftwarePackage: softwarePackage, - InstallUUID: ptr.ValOrZero(r.HostSoftwareInstallsExecutionID), - Status: "failed", - SelfService: false, - Source: source, + SoftwareTitleID: softwareTitleID, FromSetupExperience: true, } - err = svc.NewActivity(ctx, nil, activity) - if err != nil { + if err := svc.NewActivity(ctx, nil, activity); err != nil { return ctxerr.Wrap(ctx, err, "creating activity for cancelled setup experience software install") } + } else if r.VPPAppTeamID != nil { + var softwareTitleID uint + if r.SoftwareTitleID != nil { + softwareTitleID = *r.SoftwareTitleID + } + activity := fleet.ActivityTypeCanceledInstallAppStoreApp{ + HostID: hostID, + HostDisplayName: hostDisplayName, + SoftwareTitle: r.Name, + SoftwareTitleID: softwareTitleID, + FromSetupExperience: true, + } + if err := svc.NewActivity(ctx, nil, activity); err != nil { + return ctxerr.Wrap(ctx, err, "creating activity for cancelled setup experience VPP app install") + } + } + if r.IsForSoftware() { + hasCancelledSoftware = true } continue } + + // If there were cancelled software items, create a single canceled_setup_experience + // activity referencing the software that originally failed and triggered the cancellation. + if hasCancelledSoftware && failedSoftwareName != "" { + canceledActivity := fleet.ActivityTypeCanceledSetupExperience{ + HostID: hostID, + HostDisplayName: hostDisplayName, + SoftwareTitle: failedSoftwareName, + SoftwareTitleID: failedSoftwareTitleID, + } + if err := svc.NewActivity(ctx, nil, canceledActivity); err != nil { + return ctxerr.Wrap(ctx, err, "creating canceled setup experience activity") + } + } + return nil } diff --git a/ee/server/service/orbit_test.go b/ee/server/service/orbit_test.go new file mode 100644 index 00000000000..42e407aa94e --- /dev/null +++ b/ee/server/service/orbit_test.go @@ -0,0 +1,415 @@ +package service + +import ( + "context" + "log/slog" + "testing" + + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/mock" + svcmock "github.com/fleetdm/fleet/v4/server/mock/service" + "github.com/fleetdm/fleet/v4/server/ptr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFailCancelledSetupExperienceInstalls(t *testing.T) { + ctx := context.Background() + ds := new(mock.Store) + baseSvc := new(svcmock.Service) + + svc := &Service{ + Service: baseSvc, + ds: ds, + logger: slog.Default(), + } + + hostID := uint(42) + hostUUID := "host-uuid-1" + hostDisplayName := "Test Host" + + t.Run("skips non-cancelled results", func(t *testing.T) { + var activityCreated bool + baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityCreated = true + return nil + } + ds.UpdateSetupExperienceStatusResultFunc = func(ctx context.Context, status *fleet.SetupExperienceStatusResult) error { + return nil + } + + results := []*fleet.SetupExperienceStatusResult{ + { + HostUUID: hostUUID, + Status: fleet.SetupExperienceStatusPending, + SoftwareInstallerID: ptr.Uint(1), + }, + { + HostUUID: hostUUID, + Status: fleet.SetupExperienceStatusSuccess, + VPPAppTeamID: ptr.Uint(2), + }, + { + HostUUID: hostUUID, + Status: fleet.SetupExperienceStatusRunning, + VPPAppTeamID: ptr.Uint(3), + }, + { + HostUUID: hostUUID, + Status: fleet.SetupExperienceStatusFailure, + VPPAppTeamID: ptr.Uint(4), + }, + } + + err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + require.NoError(t, err) + assert.False(t, activityCreated, "no activity should be created for non-cancelled results") + assert.False(t, ds.UpdateSetupExperienceStatusResultFuncInvoked, "no update should be called for non-cancelled results") + }) + + t.Run("software package cancelled emits canceled_install_software activity with FromSetupExperience", func(t *testing.T) { + ds.UpdateSetupExperienceStatusResultFuncInvoked = false + + installerID := uint(10) + titleID := uint(100) + + ds.UpdateSetupExperienceStatusResultFunc = func(ctx context.Context, status *fleet.SetupExperienceStatusResult) error { + return nil + } + + ds.GetSoftwareInstallerMetadataByIDFunc = func(ctx context.Context, id uint) (*fleet.SoftwareInstaller, error) { + require.Equal(t, installerID, id) + return &fleet.SoftwareInstaller{ + Name: "installer.pkg", + TitleID: &titleID, + }, nil + } + + var createdActivities []fleet.ActivityDetails + var createdUser *fleet.User + baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + createdUser = user + createdActivities = append(createdActivities, activity) + return nil + } + + // The failed item that caused the cancellation + failedTitleID := uint(999) + results := []*fleet.SetupExperienceStatusResult{ + { + HostUUID: hostUUID, + Name: "FailedApp", + Status: fleet.SetupExperienceStatusFailure, + SoftwareInstallerID: ptr.Uint(99), + SoftwareTitleID: &failedTitleID, + }, + { + HostUUID: hostUUID, + Name: "DummyApp", + Status: fleet.SetupExperienceStatusCancelled, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: ptr.String("exec-uuid-1"), + }, + } + + err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + require.NoError(t, err) + + // Status should have been changed to failure + assert.Equal(t, fleet.SetupExperienceStatusFailure, results[1].Status) + + // Update should have been called + assert.True(t, ds.UpdateSetupExperienceStatusResultFuncInvoked) + + // Should have 2 activities: canceled install + canceled setup experience + require.Len(t, createdActivities, 2) + + // First: canceled install software + canceledAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledInstallSoftware) + require.True(t, ok, "expected ActivityTypeCanceledInstallSoftware, got %T", createdActivities[0]) + assert.Equal(t, hostID, canceledAct.HostID) + assert.Equal(t, hostDisplayName, canceledAct.HostDisplayName) + assert.Equal(t, "DummyApp", canceledAct.SoftwareTitle) + assert.Equal(t, titleID, canceledAct.SoftwareTitleID) + assert.True(t, canceledAct.FromSetupExperience, "FromSetupExperience should be true") + assert.True(t, canceledAct.WasFromAutomation(), "WasFromAutomation should be true") + + // Second: canceled setup experience + cseAct, ok := createdActivities[1].(fleet.ActivityTypeCanceledSetupExperience) + require.True(t, ok, "expected ActivityTypeCanceledSetupExperience, got %T", createdActivities[1]) + assert.Equal(t, hostID, cseAct.HostID) + assert.Equal(t, "FailedApp", cseAct.SoftwareTitle) + assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) + + // Should be created with nil user (Fleet-initiated) + assert.Nil(t, createdUser) + }) + + t.Run("VPP app cancelled emits canceled_install_app_store_app activity with FromSetupExperience", func(t *testing.T) { + ds.UpdateSetupExperienceStatusResultFuncInvoked = false + + vppTeamID := uint(20) + adamID := "12345" + softwareTitleID := uint(200) + + ds.UpdateSetupExperienceStatusResultFunc = func(ctx context.Context, status *fleet.SetupExperienceStatusResult) error { + return nil + } + + var createdActivities []fleet.ActivityDetails + baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + createdActivities = append(createdActivities, activity) + return nil + } + + failedTitleID := uint(888) + results := []*fleet.SetupExperienceStatusResult{ + { + HostUUID: hostUUID, + Name: "FailedVPP", + Status: fleet.SetupExperienceStatusFailure, + VPPAppTeamID: ptr.Uint(99), + SoftwareTitleID: &failedTitleID, + }, + { + HostUUID: hostUUID, + Name: "VPPApp", + Status: fleet.SetupExperienceStatusCancelled, + VPPAppTeamID: &vppTeamID, + VPPAppAdamID: &adamID, + SoftwareTitleID: &softwareTitleID, + }, + } + + err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + require.NoError(t, err) + + // Status should have been changed to failure + assert.Equal(t, fleet.SetupExperienceStatusFailure, results[1].Status) + + // Should have 2 activities: canceled VPP + canceled setup experience + require.Len(t, createdActivities, 2) + + // First: canceled install app store app + canceledAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledInstallAppStoreApp) + require.True(t, ok, "expected ActivityTypeCanceledInstallAppStoreApp, got %T", createdActivities[0]) + assert.Equal(t, hostID, canceledAct.HostID) + assert.Equal(t, hostDisplayName, canceledAct.HostDisplayName) + assert.Equal(t, "VPPApp", canceledAct.SoftwareTitle) + assert.Equal(t, softwareTitleID, canceledAct.SoftwareTitleID) + assert.True(t, canceledAct.FromSetupExperience) + assert.True(t, canceledAct.WasFromAutomation()) + + // Second: canceled setup experience + cseAct, ok := createdActivities[1].(fleet.ActivityTypeCanceledSetupExperience) + require.True(t, ok) + assert.Equal(t, "FailedVPP", cseAct.SoftwareTitle) + assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) + }) + + t.Run("mixed cancelled and non-cancelled results", func(t *testing.T) { + ds.UpdateSetupExperienceStatusResultFuncInvoked = false + + installerID := uint(30) + titleID := uint(300) + vppTeamID := uint(40) + adamID := "67890" + vppTitleID := uint(400) + + ds.UpdateSetupExperienceStatusResultFunc = func(ctx context.Context, status *fleet.SetupExperienceStatusResult) error { + return nil + } + + ds.GetSoftwareInstallerMetadataByIDFunc = func(ctx context.Context, id uint) (*fleet.SoftwareInstaller, error) { + return &fleet.SoftwareInstaller{ + Name: "installer.pkg", + TitleID: &titleID, + }, nil + } + + var activities []fleet.ActivityDetails + baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activities = append(activities, activity) + return nil + } + + failedTitleID := uint(777) + results := []*fleet.SetupExperienceStatusResult{ + { + HostUUID: hostUUID, + Name: "FailedApp", + Status: fleet.SetupExperienceStatusFailure, + SoftwareInstallerID: ptr.Uint(50), + SoftwareTitleID: &failedTitleID, + }, + { + HostUUID: hostUUID, + Name: "SuccessApp", + Status: fleet.SetupExperienceStatusSuccess, + SoftwareInstallerID: ptr.Uint(51), + }, + { + HostUUID: hostUUID, + Name: "CancelledSW", + Status: fleet.SetupExperienceStatusCancelled, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: ptr.String("exec-uuid-3"), + }, + { + HostUUID: hostUUID, + Name: "PendingVPP", + Status: fleet.SetupExperienceStatusPending, + VPPAppTeamID: ptr.Uint(60), + }, + { + HostUUID: hostUUID, + Name: "CancelledVPP", + Status: fleet.SetupExperienceStatusCancelled, + VPPAppTeamID: &vppTeamID, + VPPAppAdamID: &adamID, + SoftwareTitleID: &vppTitleID, + }, + } + + err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + require.NoError(t, err) + + // Only the two cancelled results should have their status changed + assert.Equal(t, fleet.SetupExperienceStatusFailure, results[0].Status) // was already failed + assert.Equal(t, fleet.SetupExperienceStatusSuccess, results[1].Status) // unchanged + assert.Equal(t, fleet.SetupExperienceStatusFailure, results[2].Status) // cancelled -> failed + assert.Equal(t, fleet.SetupExperienceStatusPending, results[3].Status) // unchanged + assert.Equal(t, fleet.SetupExperienceStatusFailure, results[4].Status) // cancelled -> failed + + // Three activities: canceled sw install, canceled vpp install, canceled setup experience + require.Len(t, activities, 3) + + swAct, ok := activities[0].(fleet.ActivityTypeCanceledInstallSoftware) + require.True(t, ok) + assert.Equal(t, "CancelledSW", swAct.SoftwareTitle) + assert.True(t, swAct.FromSetupExperience) + + vppAct, ok := activities[1].(fleet.ActivityTypeCanceledInstallAppStoreApp) + require.True(t, ok) + assert.Equal(t, "CancelledVPP", vppAct.SoftwareTitle) + assert.True(t, vppAct.FromSetupExperience) + + cseAct, ok := activities[2].(fleet.ActivityTypeCanceledSetupExperience) + require.True(t, ok) + assert.Equal(t, "FailedApp", cseAct.SoftwareTitle) + assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) + }) + + t.Run("script cancellation does not trigger activity", func(t *testing.T) { + ds.UpdateSetupExperienceStatusResultFuncInvoked = false + + ds.UpdateSetupExperienceStatusResultFunc = func(ctx context.Context, status *fleet.SetupExperienceStatusResult) error { + return nil + } + + var activityCreated bool + baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityCreated = true + return nil + } + + scriptID := uint(70) + results := []*fleet.SetupExperienceStatusResult{ + { + HostUUID: hostUUID, + Name: "setup.sh", + Status: fleet.SetupExperienceStatusCancelled, + SetupExperienceScriptID: &scriptID, + }, + } + + err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + require.NoError(t, err) + + // Status should still be changed to failure + assert.Equal(t, fleet.SetupExperienceStatusFailure, results[0].Status) + // But no activity should be created for script cancellations + assert.False(t, activityCreated) + }) + + t.Run("empty results returns nil", func(t *testing.T) { + err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, nil) + require.NoError(t, err) + + err = svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, []*fleet.SetupExperienceStatusResult{}) + require.NoError(t, err) + }) + + t.Run("no canceled_setup_experience activity when no failed software item", func(t *testing.T) { + ds.UpdateSetupExperienceStatusResultFuncInvoked = false + + installerID := uint(10) + titleID := uint(100) + + ds.UpdateSetupExperienceStatusResultFunc = func(ctx context.Context, status *fleet.SetupExperienceStatusResult) error { + return nil + } + + ds.GetSoftwareInstallerMetadataByIDFunc = func(ctx context.Context, id uint) (*fleet.SoftwareInstaller, error) { + return &fleet.SoftwareInstaller{ + Name: "installer.pkg", + TitleID: &titleID, + }, nil + } + + var createdActivities []fleet.ActivityDetails + baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + createdActivities = append(createdActivities, activity) + return nil + } + + // Only cancelled items, no failed item that triggered them + results := []*fleet.SetupExperienceStatusResult{ + { + HostUUID: hostUUID, + Name: "DummyApp", + Status: fleet.SetupExperienceStatusCancelled, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: ptr.String("exec-uuid-1"), + }, + } + + err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + require.NoError(t, err) + + // Should only have the canceled install, no canceled_setup_experience + require.Len(t, createdActivities, 1) + _, ok := createdActivities[0].(fleet.ActivityTypeCanceledInstallSoftware) + require.True(t, ok) + }) +} + +func TestCanceledActivityWasFromAutomation(t *testing.T) { + t.Run("CanceledInstallSoftware", func(t *testing.T) { + act := fleet.ActivityTypeCanceledInstallSoftware{ + HostID: 1, + HostDisplayName: "host", + SoftwareTitle: "title", + SoftwareTitleID: 1, + FromSetupExperience: false, + } + assert.False(t, act.WasFromAutomation()) + + act.FromSetupExperience = true + assert.True(t, act.WasFromAutomation()) + }) + + t.Run("CanceledInstallAppStoreApp", func(t *testing.T) { + act := fleet.ActivityTypeCanceledInstallAppStoreApp{ + HostID: 1, + HostDisplayName: "host", + SoftwareTitle: "title", + SoftwareTitleID: 1, + FromSetupExperience: false, + } + assert.False(t, act.WasFromAutomation()) + + act.FromSetupExperience = true + assert.True(t, act.WasFromAutomation()) + }) +} diff --git a/ee/server/service/setup_experience.go b/ee/server/service/setup_experience.go index 0ca6ff92b2a..83c2784ab0d 100644 --- a/ee/server/service/setup_experience.go +++ b/ee/server/service/setup_experience.go @@ -276,6 +276,18 @@ func (svc *Service) SetupExperienceNextStep(ctx context.Context, host *fleet.Hos svc.logger.WarnContext(ctx, "got an error when attempting to enqueue VPP app install", "err", err, "adam_id", sw.VPPAppAdamID) sw.Status = fleet.SetupExperienceStatusFailure sw.Error = ptr.String(err.Error()) + failActivity := fleet.ActivityInstalledAppStoreApp{ + HostID: host.ID, + HostDisplayName: host.DisplayName(), + SoftwareTitle: sw.Name, + AppStoreID: ptr.ValOrZero(sw.VPPAppAdamID), + Status: string(fleet.SoftwareInstallFailed), + HostPlatform: host.Platform, + FromSetupExperience: true, + } + if actErr := svc.NewActivity(ctx, nil, failActivity); actErr != nil { + svc.logger.WarnContext(ctx, "failed to create activity for VPP app install failure during setup experience", "err", actErr) + } // At this point we need to check whether the "cancel if software install fails" setting is active, // in which case we'll cancel the remaining pending items. requireAllSoftware, err := svc.IsAllSetupExperienceSoftwareRequired(ctx, host) diff --git a/frontend/interfaces/activity.ts b/frontend/interfaces/activity.ts index 7acc0cd687f..2c2d343697c 100644 --- a/frontend/interfaces/activity.ts +++ b/frontend/interfaces/activity.ts @@ -138,6 +138,7 @@ export enum ActivityType { CanceledInstallAppStoreApp = "canceled_install_app_store_app", CanceledInstallSoftware = "canceled_install_software", CanceledUninstallSoftware = "canceled_uninstall_software", + CanceledSetupExperience = "canceled_setup_experience", EnabledAndroidMdm = "enabled_android_mdm", DisabledAndroidMdm = "disabled_android_mdm", ConfiguredMSEntraConditionalAccess = "added_conditional_access_integration_microsoft", @@ -180,6 +181,7 @@ export type IHostPastActivityType = | ActivityType.CanceledInstallAppStoreApp | ActivityType.CanceledInstallSoftware | ActivityType.CanceledUninstallSoftware + | ActivityType.CanceledSetupExperience | ActivityType.InstalledCertificate | ActivityType.ResentCertificate | ActivityType.ClearedPasscode; @@ -281,6 +283,7 @@ export interface IActivityDetails { team_name?: string | null; teams?: ITeamSummary[]; triggered_by?: string; + from_setup_experience?: boolean; user_email?: string; user_id?: number; webhook_url?: string; @@ -314,6 +317,7 @@ export const ACTIVITY_TYPE_TO_FILTER_LABEL: Record = { canceled_install_software: "Canceled activity: install software", canceled_run_script: "Canceled activity: run script", canceled_uninstall_software: "Canceled activity: uninstall software", + canceled_setup_experience: "Canceled activity: macOS setup experience", changed_macos_setup_assistant: "Edited macOS automatic enrollment profile", changed_user_global_role: "Edited user's role: global", changed_user_team_role: "Edited user's role: fleet", diff --git a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx index e325b9b91bd..6d7dc30798b 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx @@ -16,6 +16,7 @@ import { formatScriptNameForActivityItem, getPerformanceImpactDescription, } from "utilities/helpers"; +import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers"; import ActivityItem from "components/ActivityItem"; import { ShowActivityDetailsHandler } from "components/ActivityItem/ActivityItem"; @@ -1497,12 +1498,30 @@ const TAGGED_TEMPLATES = { ); }, canceledInstallSoftware: (activity: IActivity) => { - const { software_title: title, host_display_name: hostName } = - activity.details || {}; + const { + software_title: title, + host_display_name: hostName, + from_setup_experience: fromSetupExperience, + } = activity.details || {}; return ( <> {" "} - canceled {title} install on {hostName}. + canceled {title} install on {hostName} + {fromSetupExperience ? " during setup experience" : ""}. + + ); + }, + canceledSetupExperience: (activity: IActivity) => { + const { + software_title: title, + software_display_name: displayName, + host_display_name: hostName, + } = activity.details || {}; + return ( + <> + {" "} + canceled setup experience on {hostName} because{" "} + {getDisplayedSoftwareName(title, displayName)} failed to install. ); }, @@ -2159,6 +2178,9 @@ const getDetail = (activity: IActivity, isPremiumTier: boolean) => { case ActivityType.CanceledUninstallSoftware: { return TAGGED_TEMPLATES.canceledUninstallSoftware(activity); } + case ActivityType.CanceledSetupExperience: { + return TAGGED_TEMPLATES.canceledSetupExperience(activity); + } case ActivityType.CreatedSavedQuery: { return TAGGED_TEMPLATES.createdSavedQuery(activity); } diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityConfig.tsx b/frontend/pages/hosts/details/cards/Activity/ActivityConfig.tsx index ac3659f7734..b1d8ac93c60 100644 --- a/frontend/pages/hosts/details/cards/Activity/ActivityConfig.tsx +++ b/frontend/pages/hosts/details/cards/Activity/ActivityConfig.tsx @@ -21,6 +21,7 @@ import RotatedHostRecoveryLockPasswordActivityItem from "./ActivityItems/Rotated import InstalledSoftwareActivityItem from "./ActivityItems/InstalledSoftwareActivityItem"; import CanceledRunScriptActivityItem from "./ActivityItems/CanceledRunScriptActivityItem"; import CanceledInstallSoftwareActivityItem from "./ActivityItems/CanceledInstallSoftwareActivityItem"; +import CanceledSetupExperienceActivityItem from "./ActivityItems/CanceledSetupExperienceActivityItem"; import CanceledUninstallSoftwareActivtyItem from "./ActivityItems/CanceledUninstallSoftwareActivtyItem"; import InstalledCertificateActivityItem from "./ActivityItems/InstalledCertificateActivityItem"; import ResentCertificateActivityItem from "./ActivityItems/ResentCertificateActivityItem"; @@ -67,6 +68,7 @@ export const pastActivityComponentMap: Record< [ActivityType.CanceledInstallSoftware]: CanceledInstallSoftwareActivityItem, [ActivityType.CanceledInstallAppStoreApp]: CanceledInstallSoftwareActivityItem, [ActivityType.CanceledUninstallSoftware]: CanceledUninstallSoftwareActivtyItem, + [ActivityType.CanceledSetupExperience]: CanceledSetupExperienceActivityItem, [ActivityType.InstalledCertificate]: InstalledCertificateActivityItem, [ActivityType.ResentCertificate]: ResentCertificateActivityItem, [ActivityType.ClearedPasscode]: ClearedPasscodeActivityItem, diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledInstallSoftwareActivityItem/CanceledInstallSoftwareActivityItem.tsx b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledInstallSoftwareActivityItem/CanceledInstallSoftwareActivityItem.tsx index b83358eaeeb..acbb7daa155 100644 --- a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledInstallSoftwareActivityItem/CanceledInstallSoftwareActivityItem.tsx +++ b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledInstallSoftwareActivityItem/CanceledInstallSoftwareActivityItem.tsx @@ -9,6 +9,8 @@ const baseClass = "canceled-install-software-activity-item"; const CanceledInstallSoftwareActivityItem = ({ activity, }: IHostActivityItemComponentProps) => { + const fromSetupExperience = activity.details?.from_setup_experience; + return ( {" "} - install on this host. + install on this host + {fromSetupExperience ? " during setup experience" : ""}. ); diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx new file mode 100644 index 00000000000..efcf11826b6 --- /dev/null +++ b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx @@ -0,0 +1,35 @@ +import React from "react"; + +import ActivityItem from "components/ActivityItem"; +import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers"; + +import { IHostActivityItemComponentProps } from "../../ActivityConfig"; + +const baseClass = "canceled-setup-experience-activity-item"; + +const CanceledSetupExperienceActivityItem = ({ + activity, +}: IHostActivityItemComponentProps) => { + return ( + + <> + {activity.actor_full_name} canceled setup experience on this host + because{" "} + + {getDisplayedSoftwareName( + activity.details.software_title, + activity.details.software_display_name + )} + {" "} + failed to install. + + + ); +}; + +export default CanceledSetupExperienceActivityItem; diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/index.ts b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/index.ts new file mode 100644 index 00000000000..63a40685a0d --- /dev/null +++ b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/index.ts @@ -0,0 +1 @@ +export { default } from "./CanceledSetupExperienceActivityItem"; diff --git a/server/fleet/activities.go b/server/fleet/activities.go index 38a7e62542d..a94e25be782 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -233,6 +233,8 @@ var ActivityDetailsList = []ActivityDetails{ ActivityTypeEditedHostIdpData{}, ActivityTypeEditedEnrollSecrets{}, + + ActivityTypeCanceledSetupExperience{}, } // ActivityDetails is an alias for the canonical ActivityDetails interface defined in server/activity/api. @@ -1541,10 +1543,11 @@ func (a ActivityTypeCanceledRunScript) HostIDs() []uint { } type ActivityTypeCanceledInstallSoftware struct { - HostID uint `json:"host_id"` - HostDisplayName string `json:"host_display_name"` - SoftwareTitle string `json:"software_title"` - SoftwareTitleID uint `json:"software_title_id"` + HostID uint `json:"host_id"` + HostDisplayName string `json:"host_display_name"` + SoftwareTitle string `json:"software_title"` + SoftwareTitleID uint `json:"software_title_id"` + FromSetupExperience bool `json:"from_setup_experience"` } func (a ActivityTypeCanceledInstallSoftware) ActivityName() string { @@ -1555,6 +1558,10 @@ func (a ActivityTypeCanceledInstallSoftware) HostIDs() []uint { return []uint{a.HostID} } +func (a ActivityTypeCanceledInstallSoftware) WasFromAutomation() bool { + return a.FromSetupExperience +} + type ActivityTypeCanceledUninstallSoftware struct { HostID uint `json:"host_id"` HostDisplayName string `json:"host_display_name"` @@ -1571,10 +1578,11 @@ func (a ActivityTypeCanceledUninstallSoftware) HostIDs() []uint { } type ActivityTypeCanceledInstallAppStoreApp struct { - HostID uint `json:"host_id"` - HostDisplayName string `json:"host_display_name"` - SoftwareTitle string `json:"software_title"` - SoftwareTitleID uint `json:"software_title_id"` + HostID uint `json:"host_id"` + HostDisplayName string `json:"host_display_name"` + SoftwareTitle string `json:"software_title"` + SoftwareTitleID uint `json:"software_title_id"` + FromSetupExperience bool `json:"from_setup_experience"` } func (a ActivityTypeCanceledInstallAppStoreApp) HostIDs() []uint { @@ -1585,6 +1593,10 @@ func (a ActivityTypeCanceledInstallAppStoreApp) ActivityName() string { return "canceled_install_app_store_app" } +func (a ActivityTypeCanceledInstallAppStoreApp) WasFromAutomation() bool { + return a.FromSetupExperience +} + type ActivityTypeRanScriptBatch struct { ScriptName string `json:"script_name"` BatchExecutionID string `json:"batch_execution_id"` @@ -1869,3 +1881,18 @@ func (a ActivityTypeClearedPasscode) HostIDs() []uint { func (a ActivityTypeClearedPasscode) HostOnly() bool { return true } + +type ActivityTypeCanceledSetupExperience struct { + HostID uint `json:"host_id"` + HostDisplayName string `json:"host_display_name"` + SoftwareTitle string `json:"software_title"` + SoftwareTitleID uint `json:"software_title_id"` +} + +func (a ActivityTypeCanceledSetupExperience) ActivityName() string { + return "canceled_setup_experience" +} + +func (a ActivityTypeCanceledSetupExperience) HostIDs() []uint { + return []uint{a.HostID} +} diff --git a/server/worker/apple_mdm.go b/server/worker/apple_mdm.go index 5bc9dbf04be..a8a4a9cacf7 100644 --- a/server/worker/apple_mdm.go +++ b/server/worker/apple_mdm.go @@ -46,6 +46,7 @@ type AppleMDM struct { Commander *apple_mdm.MDMAppleCommander BootstrapPackageStore fleet.MDMBootstrapPackageStore VPPInstaller fleet.AppleMDMVPPInstaller + NewActivityFn fleet.NewActivityFunc } // Name returns the name of the job. @@ -590,6 +591,21 @@ func (a *AppleMDM) installSetupExperienceVPPAppsOnIosIpadOS(ctx context.Context, a.Log.ErrorContext(ctx, "got an error when attempting to enqueue VPP app install", "err", err, "adam_id", app.VPPAppAdamID) app.Status = fleet.SetupExperienceStatusFailure app.Error = ptr.String(err.Error()) + // Emit activity for the VPP app install failure + if a.NewActivityFn != nil { + failActivity := fleet.ActivityInstalledAppStoreApp{ + HostID: host.ID, + HostDisplayName: host.DisplayName(), + SoftwareTitle: app.Name, + AppStoreID: ptr.ValOrZero(app.VPPAppAdamID), + Status: string(fleet.SoftwareInstallFailed), + HostPlatform: host.Platform, + FromSetupExperience: true, + } + if actErr := a.NewActivityFn(ctx, nil, failActivity); actErr != nil { + a.Log.WarnContext(ctx, "failed to create activity for VPP app install failure during setup experience", "err", actErr) + } + } } else { app.NanoCommandUUID = &cmdUUID app.Status = fleet.SetupExperienceStatusRunning From ad3112f13e91a8015328e86c0109ca760cb74854 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Fri, 10 Apr 2026 20:15:45 -0500 Subject: [PATCH 02/41] Tweak changes file --- changes/34288-setup-experience-cancel-activity | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/changes/34288-setup-experience-cancel-activity b/changes/34288-setup-experience-cancel-activity index 4fd6269c41c..59933776529 100644 --- a/changes/34288-setup-experience-cancel-activity +++ b/changes/34288-setup-experience-cancel-activity @@ -1,4 +1,3 @@ -* Added "Canceled activity: macOS setup experience" activity when setup experience is cancelled due to a software install failure. -* Added cancel activities for VPP app installs that are skipped when setup experience is cancelled. -* Added `from_setup_experience` flag to cancelled software install activities during setup experience. -* Added install failure activity when VPP app installs fail prior to sending the MDM command during setup experience. +* Added activity when setup experience is cancelled due to software install failure +* Added cancel activities for each VPP app install skipped due to setup experience cancellation, and switched "failed" activity to "cancelled" for package-based software installs in the same situation +* Added install failure activity when VPP installs fail due to licensing issues during setup experience From f5979757b23d2d838bcc286511f7aa9291fa32ca Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Fri, 10 Apr 2026 20:24:52 -0500 Subject: [PATCH 03/41] Match filter label to Figma --- frontend/interfaces/activity.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/interfaces/activity.ts b/frontend/interfaces/activity.ts index 2c2d343697c..e7e74b4f212 100644 --- a/frontend/interfaces/activity.ts +++ b/frontend/interfaces/activity.ts @@ -317,7 +317,7 @@ export const ACTIVITY_TYPE_TO_FILTER_LABEL: Record = { canceled_install_software: "Canceled activity: install software", canceled_run_script: "Canceled activity: run script", canceled_uninstall_software: "Canceled activity: uninstall software", - canceled_setup_experience: "Canceled activity: macOS setup experience", + canceled_setup_experience: "Canceled setup experience", changed_macos_setup_assistant: "Edited macOS automatic enrollment profile", changed_user_global_role: "Edited user's role: global", changed_user_team_role: "Edited user's role: fleet", From 5fbe3feb8747de528da90fab566dded9418c5a5f Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Fri, 10 Apr 2026 20:31:03 -0500 Subject: [PATCH 04/41] Add "during setup experience" copy when relevant on installs --- .../ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx index 6d7dc30798b..6158f45ee3c 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx @@ -1282,6 +1282,7 @@ const TAGGED_TEMPLATES = { software_title: title, status, source, + from_setup_experience, } = details; const showSoftwarePackage = @@ -1294,7 +1295,8 @@ const TAGGED_TEMPLATES = { {getInstallUninstallStatusPredicate(status, isScriptPackageSource)}{" "} {title} {showSoftwarePackage && ` (${details.software_package})`} on{" "} - {hostName}. + {hostName} + {from_setup_experience ? " during setup experience" : ""}. ); }, From 4f35a80a3a2f28c296872ad6fc823b7ec3e72561 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Fri, 10 Apr 2026 21:05:29 -0500 Subject: [PATCH 05/41] =?UTF-8?q?=F0=9F=A4=96=20Add=20test=20coverage=20fo?= =?UTF-8?q?r=20VPP=20install=20failure=20during=20setup=20experience=20act?= =?UTF-8?q?ivity=20emission?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: Are there tests covering the new activity creation in `server/worker/apple_mdm.go` L594-608? If not, add test coverage to assert that the activity is as expected. --- server/worker/apple_mdm_test.go | 65 +++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/server/worker/apple_mdm_test.go b/server/worker/apple_mdm_test.go index 06b79f6ce7f..44919de19aa 100644 --- a/server/worker/apple_mdm_test.go +++ b/server/worker/apple_mdm_test.go @@ -1351,6 +1351,71 @@ INSERT INTO setup_experience_status_results ( require.Contains(t, getEnqueuedCommandTypes(t), "DeviceConfigured") }) + t.Run("emits activity on VPP install failure during setup experience", func(t *testing.T) { + mysql.SetTestABMAssets(t, ds, testOrgName) + test.CreateInsertGlobalVPPToken(t, ds) + defer mysql.TruncateTables(t, ds) + + tm, err := ds.NewTeam(ctx, &fleet.Team{Name: "test"}) + require.NoError(t, err) + + h := createEnrolledHost(t, 1, &tm.ID, true, "ios") + + vppApp := &fleet.VPPApp{ + Name: "fail-app", LatestVersion: "1.0.0", + VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "fail-adam-id", Platform: fleet.IOSPlatform}}, + BundleIdentifier: "com.example.fail", + } + vppAppWithTeam, err := ds.InsertVPPAppWithTeam(ctx, vppApp, &tm.ID) + require.NoError(t, err) + + mysql.ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err = q.ExecContext(ctx, ` +INSERT INTO setup_experience_status_results (host_uuid, name, status, vpp_app_team_id) +VALUES (?, ?, ?, ?)`, h.UUID, vppAppWithTeam.Name, fleet.SetupExperienceStatusPending, vppAppWithTeam.VPPAppTeam.AppTeamID) + return err + }) + + appInstallResponses := map[string]installAppResponse{ + vppAppWithTeam.AdamID: {CommandUUID: "bad-cmd", Error: errors.New("no available licenses")}, + } + vppInstaller := &mockVPPInstaller{t: t, ds: ds, appInstallResponses: appInstallResponses} + + var capturedActivities []fleet.ActivityDetails + mdmWorker := &AppleMDM{ + VPPInstaller: vppInstaller, + Datastore: ds, + Log: slogLog, + Commander: apple_mdm.NewMDMAppleCommander(mdmStorage, mockPusher{}), + NewActivityFn: func(_ context.Context, _ *fleet.User, activity fleet.ActivityDetails) error { + capturedActivities = append(capturedActivities, activity) + return nil + }, + } + w := NewWorker(ds, slogLog) + w.Register(mdmWorker) + + err = QueueAppleMDMJob(ctx, ds, slogLog, AppleMDMPostDEPEnrollmentTask, h.UUID, h.Platform, nil, "", true, false) + require.NoError(t, err) + + err = w.ProcessJobs(ctx) + require.NoError(t, err) + + // Exactly one activity should have been emitted for the failed VPP install + require.Len(t, capturedActivities, 1) + act, ok := capturedActivities[0].(fleet.ActivityInstalledAppStoreApp) + require.True(t, ok, "expected ActivityInstalledAppStoreApp, got %T", capturedActivities[0]) + + assert.Equal(t, h.ID, act.HostID) + assert.Equal(t, h.DisplayName(), act.HostDisplayName) + assert.Equal(t, vppAppWithTeam.Name, act.SoftwareTitle) + assert.Equal(t, vppAppWithTeam.AdamID, act.AppStoreID) + assert.Equal(t, string(fleet.SoftwareInstallFailed), act.Status) + assert.Equal(t, h.Platform, act.HostPlatform) + assert.True(t, act.FromSetupExperience) + assert.False(t, act.SelfService) + }) + t.Run("treats NotNow status as a finished command status that does not block device release", func(t *testing.T) { mysql.SetTestABMAssets(t, ds, testOrgName) defer mysql.TruncateTables(t, ds) From 5ef9c8db8def24fa0994cbc9ec8235048dff91bc Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 14:35:33 -0500 Subject: [PATCH 06/41] Clean up cancellation logic --- ee/server/service/devices.go | 4 +- ee/server/service/orbit.go | 74 +++++++++----------------------- ee/server/service/orbit_test.go | 16 +++---- server/fleet/setup_experience.go | 4 ++ 4 files changed, 35 insertions(+), 63 deletions(-) diff --git a/ee/server/service/devices.go b/ee/server/service/devices.go index ce2c9d32dda..de84d9a287f 100644 --- a/ee/server/service/devices.go +++ b/ee/server/service/devices.go @@ -319,8 +319,8 @@ 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") } diff --git a/ee/server/service/orbit.go b/ee/server/service/orbit.go index d144b2f1bb4..22a3055d284 100644 --- a/ee/server/service/orbit.go +++ b/ee/server/service/orbit.go @@ -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{ @@ -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, @@ -237,82 +236,51 @@ func (svc *Service) failCancelledSetupExperienceInstalls( results []*fleet.SetupExperienceStatusResult, ) error { // Find the software item that originally failed and triggered the cancellation. - // It already has SetupExperienceStatusFailure before we modify any cancelled items. - var failedSoftwareName string - var failedSoftwareTitleID uint + // It already has SetupExperienceStatusFailure before we modify any canceled items. for _, r := range results { if r.Status == fleet.SetupExperienceStatusFailure && r.IsForSoftware() { - failedSoftwareName = r.Name - if r.SoftwareTitleID != nil { - failedSoftwareTitleID = *r.SoftwareTitleID + if err := svc.NewActivity(ctx, nil, fleet.ActivityTypeCanceledSetupExperience{ + HostID: hostID, + HostDisplayName: hostDisplayName, + SoftwareTitle: r.Name, + SoftwareTitleID: ptr.ValOrZero(r.SoftwareTitleID), + }); err != nil { + return ctxerr.Wrap(ctx, err, "creating canceled setup experience activity") } break } } - var hasCancelledSoftware bool for _, r := range results { if r.Status != fleet.SetupExperienceStatusCancelled { 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, "marking setup experience software as cancelled", "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") } if r.IsForSoftwarePackage() { - var softwareTitleID uint - 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 && installerMeta.TitleID != nil { - softwareTitleID = *installerMeta.TitleID - } - activity := fleet.ActivityTypeCanceledInstallSoftware{ + if err := svc.NewActivity(ctx, nil, fleet.ActivityTypeCanceledInstallSoftware{ HostID: hostID, HostDisplayName: hostDisplayName, SoftwareTitle: r.Name, - SoftwareTitleID: softwareTitleID, + SoftwareTitleID: ptr.ValOrZero(r.SoftwareTitleID), FromSetupExperience: true, + }); err != nil { + return ctxerr.Wrap(ctx, err, "creating activity for canceled setup experience software install") } - if err := svc.NewActivity(ctx, nil, activity); err != nil { - return ctxerr.Wrap(ctx, err, "creating activity for cancelled setup experience software install") - } - } else if r.VPPAppTeamID != nil { - var softwareTitleID uint - if r.SoftwareTitleID != nil { - softwareTitleID = *r.SoftwareTitleID - } - activity := fleet.ActivityTypeCanceledInstallAppStoreApp{ + } else if r.IsForVPPApp() { + if err := svc.NewActivity(ctx, nil, fleet.ActivityTypeCanceledInstallAppStoreApp{ HostID: hostID, HostDisplayName: hostDisplayName, SoftwareTitle: r.Name, - SoftwareTitleID: softwareTitleID, + SoftwareTitleID: ptr.ValOrZero(r.SoftwareTitleID), FromSetupExperience: true, + }); err != nil { + return ctxerr.Wrap(ctx, err, "creating activity for canceled setup experience VPP app install") } - if err := svc.NewActivity(ctx, nil, activity); err != nil { - return ctxerr.Wrap(ctx, err, "creating activity for cancelled setup experience VPP app install") - } - } - if r.IsForSoftware() { - hasCancelledSoftware = true - } - continue - } - - // If there were cancelled software items, create a single canceled_setup_experience - // activity referencing the software that originally failed and triggered the cancellation. - if hasCancelledSoftware && failedSoftwareName != "" { - canceledActivity := fleet.ActivityTypeCanceledSetupExperience{ - HostID: hostID, - HostDisplayName: hostDisplayName, - SoftwareTitle: failedSoftwareName, - SoftwareTitleID: failedSoftwareTitleID, - } - if err := svc.NewActivity(ctx, nil, canceledActivity); err != nil { - return ctxerr.Wrap(ctx, err, "creating canceled setup experience activity") } } diff --git a/ee/server/service/orbit_test.go b/ee/server/service/orbit_test.go index 42e407aa94e..3a012f58afc 100644 --- a/ee/server/service/orbit_test.go +++ b/ee/server/service/orbit_test.go @@ -61,7 +61,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { }, } - err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + err := svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, results) require.NoError(t, err) assert.False(t, activityCreated, "no activity should be created for non-cancelled results") assert.False(t, ds.UpdateSetupExperienceStatusResultFuncInvoked, "no update should be called for non-cancelled results") @@ -112,7 +112,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { }, } - err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + err := svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, results) require.NoError(t, err) // Status should have been changed to failure @@ -181,7 +181,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { }, } - err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + err := svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, results) require.NoError(t, err) // Status should have been changed to failure @@ -271,7 +271,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { }, } - err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + err := svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, results) require.NoError(t, err) // Only the two cancelled results should have their status changed @@ -323,7 +323,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { }, } - err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + err := svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, results) require.NoError(t, err) // Status should still be changed to failure @@ -333,10 +333,10 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { }) t.Run("empty results returns nil", func(t *testing.T) { - err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, nil) + err := svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, nil) require.NoError(t, err) - err = svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, []*fleet.SetupExperienceStatusResult{}) + err = svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, []*fleet.SetupExperienceStatusResult{}) require.NoError(t, err) }) @@ -374,7 +374,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { }, } - err := svc.failCancelledSetupExperienceInstalls(ctx, hostID, hostUUID, hostDisplayName, results) + err := svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, results) require.NoError(t, err) // Should only have the canceled install, no canceled_setup_experience diff --git a/server/fleet/setup_experience.go b/server/fleet/setup_experience.go index 75912134e3f..b52aed3b89c 100644 --- a/server/fleet/setup_experience.go +++ b/server/fleet/setup_experience.go @@ -121,6 +121,10 @@ func (s *SetupExperienceStatusResult) IsForSoftwarePackage() bool { return s.SoftwareInstallerID != nil } +func (s *SetupExperienceStatusResult) IsForVPPApp() bool { + return s.VPPAppTeamID != nil +} + func (s *SetupExperienceStatusResult) ForMyDevicePage(token string) { // convert api style iconURL to device token URL if s.IconURL != "" && s.SoftwareTitleID != nil { From 125699d5207996252eef4aea618cadc360ea81a9 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 14:42:15 -0500 Subject: [PATCH 07/41] =?UTF-8?q?=F0=9F=A4=96=20Catch=20up=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opis 4.6; prompt: Update tests added on this branch to reflect the cleanup done in the most recent commit. --- ee/server/service/orbit_test.go | 90 +++++++++++++-------------------- 1 file changed, 36 insertions(+), 54 deletions(-) diff --git a/ee/server/service/orbit_test.go b/ee/server/service/orbit_test.go index 3a012f58afc..7b2e4905c04 100644 --- a/ee/server/service/orbit_test.go +++ b/ee/server/service/orbit_test.go @@ -55,9 +55,9 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { VPPAppTeamID: ptr.Uint(3), }, { - HostUUID: hostUUID, - Status: fleet.SetupExperienceStatusFailure, - VPPAppTeamID: ptr.Uint(4), + HostUUID: hostUUID, + Status: fleet.SetupExperienceStatusFailure, + SetupExperienceScriptID: ptr.Uint(4), }, } @@ -77,14 +77,6 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { return nil } - ds.GetSoftwareInstallerMetadataByIDFunc = func(ctx context.Context, id uint) (*fleet.SoftwareInstaller, error) { - require.Equal(t, installerID, id) - return &fleet.SoftwareInstaller{ - Name: "installer.pkg", - TitleID: &titleID, - }, nil - } - var createdActivities []fleet.ActivityDetails var createdUser *fleet.User baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { @@ -108,6 +100,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { Name: "DummyApp", Status: fleet.SetupExperienceStatusCancelled, SoftwareInstallerID: &installerID, + SoftwareTitleID: &titleID, HostSoftwareInstallsExecutionID: ptr.String("exec-uuid-1"), }, } @@ -121,12 +114,19 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { // Update should have been called assert.True(t, ds.UpdateSetupExperienceStatusResultFuncInvoked) - // Should have 2 activities: canceled install + canceled setup experience + // Should have 2 activities: canceled setup experience + canceled install require.Len(t, createdActivities, 2) - // First: canceled install software - canceledAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledInstallSoftware) - require.True(t, ok, "expected ActivityTypeCanceledInstallSoftware, got %T", createdActivities[0]) + // First: canceled setup experience (emitted in first loop for the failed item) + cseAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledSetupExperience) + require.True(t, ok, "expected ActivityTypeCanceledSetupExperience, got %T", createdActivities[0]) + assert.Equal(t, hostID, cseAct.HostID) + assert.Equal(t, "FailedApp", cseAct.SoftwareTitle) + assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) + + // Second: canceled install software + canceledAct, ok := createdActivities[1].(fleet.ActivityTypeCanceledInstallSoftware) + require.True(t, ok, "expected ActivityTypeCanceledInstallSoftware, got %T", createdActivities[1]) assert.Equal(t, hostID, canceledAct.HostID) assert.Equal(t, hostDisplayName, canceledAct.HostDisplayName) assert.Equal(t, "DummyApp", canceledAct.SoftwareTitle) @@ -134,13 +134,6 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { assert.True(t, canceledAct.FromSetupExperience, "FromSetupExperience should be true") assert.True(t, canceledAct.WasFromAutomation(), "WasFromAutomation should be true") - // Second: canceled setup experience - cseAct, ok := createdActivities[1].(fleet.ActivityTypeCanceledSetupExperience) - require.True(t, ok, "expected ActivityTypeCanceledSetupExperience, got %T", createdActivities[1]) - assert.Equal(t, hostID, cseAct.HostID) - assert.Equal(t, "FailedApp", cseAct.SoftwareTitle) - assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) - // Should be created with nil user (Fleet-initiated) assert.Nil(t, createdUser) }) @@ -187,24 +180,25 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { // Status should have been changed to failure assert.Equal(t, fleet.SetupExperienceStatusFailure, results[1].Status) - // Should have 2 activities: canceled VPP + canceled setup experience + // Should have 2 activities: canceled setup experience + canceled VPP install require.Len(t, createdActivities, 2) - // First: canceled install app store app - canceledAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledInstallAppStoreApp) - require.True(t, ok, "expected ActivityTypeCanceledInstallAppStoreApp, got %T", createdActivities[0]) + // First: canceled setup experience (emitted in first loop for the failed item) + cseAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledSetupExperience) + require.True(t, ok, "expected ActivityTypeCanceledSetupExperience, got %T", createdActivities[0]) + assert.Equal(t, hostID, cseAct.HostID) + assert.Equal(t, "FailedVPP", cseAct.SoftwareTitle) + assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) + + // Second: canceled install app store app + canceledAct, ok := createdActivities[1].(fleet.ActivityTypeCanceledInstallAppStoreApp) + require.True(t, ok, "expected ActivityTypeCanceledInstallAppStoreApp, got %T", createdActivities[1]) assert.Equal(t, hostID, canceledAct.HostID) assert.Equal(t, hostDisplayName, canceledAct.HostDisplayName) assert.Equal(t, "VPPApp", canceledAct.SoftwareTitle) assert.Equal(t, softwareTitleID, canceledAct.SoftwareTitleID) assert.True(t, canceledAct.FromSetupExperience) assert.True(t, canceledAct.WasFromAutomation()) - - // Second: canceled setup experience - cseAct, ok := createdActivities[1].(fleet.ActivityTypeCanceledSetupExperience) - require.True(t, ok) - assert.Equal(t, "FailedVPP", cseAct.SoftwareTitle) - assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) }) t.Run("mixed cancelled and non-cancelled results", func(t *testing.T) { @@ -220,13 +214,6 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { return nil } - ds.GetSoftwareInstallerMetadataByIDFunc = func(ctx context.Context, id uint) (*fleet.SoftwareInstaller, error) { - return &fleet.SoftwareInstaller{ - Name: "installer.pkg", - TitleID: &titleID, - }, nil - } - var activities []fleet.ActivityDetails baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { activities = append(activities, activity) @@ -253,6 +240,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { Name: "CancelledSW", Status: fleet.SetupExperienceStatusCancelled, SoftwareInstallerID: &installerID, + SoftwareTitleID: &titleID, HostSoftwareInstallsExecutionID: ptr.String("exec-uuid-3"), }, { @@ -281,23 +269,23 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { assert.Equal(t, fleet.SetupExperienceStatusPending, results[3].Status) // unchanged assert.Equal(t, fleet.SetupExperienceStatusFailure, results[4].Status) // cancelled -> failed - // Three activities: canceled sw install, canceled vpp install, canceled setup experience + // Three activities: canceled setup experience, canceled sw install, canceled vpp install require.Len(t, activities, 3) - swAct, ok := activities[0].(fleet.ActivityTypeCanceledInstallSoftware) + cseAct, ok := activities[0].(fleet.ActivityTypeCanceledSetupExperience) + require.True(t, ok) + assert.Equal(t, "FailedApp", cseAct.SoftwareTitle) + assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) + + swAct, ok := activities[1].(fleet.ActivityTypeCanceledInstallSoftware) require.True(t, ok) assert.Equal(t, "CancelledSW", swAct.SoftwareTitle) assert.True(t, swAct.FromSetupExperience) - vppAct, ok := activities[1].(fleet.ActivityTypeCanceledInstallAppStoreApp) + vppAct, ok := activities[2].(fleet.ActivityTypeCanceledInstallAppStoreApp) require.True(t, ok) assert.Equal(t, "CancelledVPP", vppAct.SoftwareTitle) assert.True(t, vppAct.FromSetupExperience) - - cseAct, ok := activities[2].(fleet.ActivityTypeCanceledSetupExperience) - require.True(t, ok) - assert.Equal(t, "FailedApp", cseAct.SoftwareTitle) - assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) }) t.Run("script cancellation does not trigger activity", func(t *testing.T) { @@ -350,13 +338,6 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { return nil } - ds.GetSoftwareInstallerMetadataByIDFunc = func(ctx context.Context, id uint) (*fleet.SoftwareInstaller, error) { - return &fleet.SoftwareInstaller{ - Name: "installer.pkg", - TitleID: &titleID, - }, nil - } - var createdActivities []fleet.ActivityDetails baseSvc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { createdActivities = append(createdActivities, activity) @@ -370,6 +351,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { Name: "DummyApp", Status: fleet.SetupExperienceStatusCancelled, SoftwareInstallerID: &installerID, + SoftwareTitleID: &titleID, HostSoftwareInstallsExecutionID: ptr.String("exec-uuid-1"), }, } From 17b2c1c2a66be4ac80b9a77c9103c1cf88f48e1f Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 15:15:27 -0500 Subject: [PATCH 08/41] =?UTF-8?q?=F0=9F=A4=96=20FIx=20more=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: Resolve test failures in `TestIntegrationsEnterprise/TestSetupExperienceLinuxWithSoftware` and `TestIntegrationsMDM/TestCancelUpcomingActivity`. You can enable MySQL tests to validate that tests run correctly; don't guess based on code analysis. --- server/service/integration_enterprise_test.go | 13 ++++--------- server/service/integration_mdm_test.go | 4 ++-- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 603ac5ca9da..65774710605 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -23331,18 +23331,13 @@ func (s *integrationEnterpriseTestSuite) TestSetupExperienceLinuxWithSoftware() require.NotEmpty(t, executionIDs["vim"]) require.NotEmpty(t, executionIDs["test.tar.gz"]) - s.lastActivityOfTypeMatches(fleet.ActivityTypeInstalledSoftware{}.ActivityName(), fmt.Sprintf(`{ + s.lastActivityOfTypeMatches(fleet.ActivityTypeCanceledInstallSoftware{}.ActivityName(), fmt.Sprintf(`{ "host_id": %d, "host_display_name": %q, "software_title": %q, - "software_package": %q, - "install_uuid": %q, - "status": "failed", - "self_service": false, - "source": "deb_packages", - "policy_name": null, - "policy_id": null - }`, ubuntuHost.ID, ubuntuHost.DisplayName(), "vim", "vim.deb", executionIDs["vim"]), 0) + "software_title_id": %d, + "from_setup_experience": true + }`, ubuntuHost.ID, ubuntuHost.DisplayName(), "vim", debVimTitleID), 0) // Record a result for test.tar.gz. s.Do("POST", "/api/fleet/orbit/software_install/result", json.RawMessage( diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 2d707ee1546..278e33d4109 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -19699,7 +19699,7 @@ func (s *integrationMDMTestSuite) TestCancelUpcomingActivity() { // cancel the VPP app install, confirm canceled activity s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/hosts/%d/activities/upcoming/%s", mdmHost.ID, hostActivitiesResp.Activities[0].UUID), nil, http.StatusNoContent) lastCanceledActID = s.lastActivityOfTypeMatches(fleet.ActivityTypeCanceledInstallAppStoreApp{}.ActivityName(), - fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "software_title": "App 1", "software_title_id": %d}`, mdmHost.ID, mdmHost.DisplayName(), vppAppTitleID), 0) + fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "software_title": "App 1", "software_title_id": %d, "from_setup_experience": false}`, mdmHost.ID, mdmHost.DisplayName(), vppAppTitleID), 0) // to be able to simulate the host sending a MDM result post-cancelation, // we need to re-activate the MDM command. @@ -19733,7 +19733,7 @@ func (s *integrationMDMTestSuite) TestCancelUpcomingActivity() { // cancel the software install, confirm canceled activity s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/hosts/%d/activities/upcoming/%s", mdmHost.ID, hostActivitiesResp.Activities[1].UUID), nil, http.StatusNoContent) lastCanceledActID = s.lastActivityOfTypeMatches(fleet.ActivityTypeCanceledInstallSoftware{}.ActivityName(), - fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "software_title": "DummyApp", "software_title_id": %d}`, mdmHost.ID, mdmHost.DisplayName(), swTitleID), 0) + fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "software_title": "DummyApp", "software_title_id": %d, "from_setup_experience": false}`, mdmHost.ID, mdmHost.DisplayName(), swTitleID), 0) // record a software install result post-cancelation s.Do("POST", "/api/fleet/orbit/software_install/result", json.RawMessage(fmt.Sprintf(`{ From 662d1920fe62254f4f367cb2bb99528b030872c2 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 20:42:44 -0500 Subject: [PATCH 09/41] Spelling consistency (even though I personally use "cancelled") --- ee/server/service/orbit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/server/service/orbit.go b/ee/server/service/orbit.go index 22a3055d284..220ea3626b5 100644 --- a/ee/server/service/orbit.go +++ b/ee/server/service/orbit.go @@ -256,7 +256,7 @@ func (svc *Service) recordCanceledSetupExperienceSoftwareActivities( continue } r.Status = fleet.SetupExperienceStatusFailure - svc.logger.InfoContext(ctx, "marking setup experience software as cancelled", "host_uuid", hostUUID, "software_name", r.Name) + svc.logger.InfoContext(ctx, "marking setup experience software as canceled", "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") From 25dbf52bdf5f3dc234f7ccac15d1e284dea2dce5 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 22:46:38 -0500 Subject: [PATCH 10/41] Mark setup experience cancellations as from automation Resolves https://github.com/fleetdm/fleet/pull/43437#discussion_r3070662597 --- server/fleet/activities.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/fleet/activities.go b/server/fleet/activities.go index a94e25be782..8dd4ae08f70 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -1896,3 +1896,7 @@ func (a ActivityTypeCanceledSetupExperience) ActivityName() string { func (a ActivityTypeCanceledSetupExperience) HostIDs() []uint { return []uint{a.HostID} } + +func (a ActivityTypeCanceledSetupExperience) WasFromAutomation() bool { + return true +} From b755699f039395d0b78c31c4c524e4b22f21f0ca Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 23:21:28 -0500 Subject: [PATCH 11/41] =?UTF-8?q?=F0=9F=A4=96=20Move=20activity=20emission?= =?UTF-8?q?=20for=20setup=20experience=20cxl=20to=20when=20triggering=20in?= =?UTF-8?q?stall=20fails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: Move L238-252 to L~293 of maybeCancelPendingSetupExperienceSteps, wiring up dependencies as needed. This looks to be the only way to resolve https://github.com/fleetdm/fleet/pull/43437#discussion_r3070662601 since otherwise we would emit an activity on each poll. It may make sense to pass through the relevant failed install informaiton rather than pulling the failure out of the database. --- ee/server/service/orbit.go | 16 -------- ee/server/service/orbit_test.go | 53 ++++++++----------------- ee/server/service/setup_experience.go | 12 ++++-- server/service/apple_mdm.go | 2 +- server/service/apple_mdm_cmd_results.go | 2 +- server/service/orbit.go | 4 +- server/service/setup_experience.go | 28 +++++++++++-- server/service/setup_experience_test.go | 12 +++--- 8 files changed, 60 insertions(+), 69 deletions(-) diff --git a/ee/server/service/orbit.go b/ee/server/service/orbit.go index 220ea3626b5..3abe1d34887 100644 --- a/ee/server/service/orbit.go +++ b/ee/server/service/orbit.go @@ -235,22 +235,6 @@ func (svc *Service) recordCanceledSetupExperienceSoftwareActivities( hostDisplayName string, results []*fleet.SetupExperienceStatusResult, ) error { - // Find the software item that originally failed and triggered the cancellation. - // It already has SetupExperienceStatusFailure before we modify any canceled items. - for _, r := range results { - if r.Status == fleet.SetupExperienceStatusFailure && r.IsForSoftware() { - if err := svc.NewActivity(ctx, nil, fleet.ActivityTypeCanceledSetupExperience{ - HostID: hostID, - HostDisplayName: hostDisplayName, - SoftwareTitle: r.Name, - SoftwareTitleID: ptr.ValOrZero(r.SoftwareTitleID), - }); err != nil { - return ctxerr.Wrap(ctx, err, "creating canceled setup experience activity") - } - break - } - } - for _, r := range results { if r.Status != fleet.SetupExperienceStatusCancelled { continue diff --git a/ee/server/service/orbit_test.go b/ee/server/service/orbit_test.go index 7b2e4905c04..43fc54201b8 100644 --- a/ee/server/service/orbit_test.go +++ b/ee/server/service/orbit_test.go @@ -114,19 +114,12 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { // Update should have been called assert.True(t, ds.UpdateSetupExperienceStatusResultFuncInvoked) - // Should have 2 activities: canceled setup experience + canceled install - require.Len(t, createdActivities, 2) - - // First: canceled setup experience (emitted in first loop for the failed item) - cseAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledSetupExperience) - require.True(t, ok, "expected ActivityTypeCanceledSetupExperience, got %T", createdActivities[0]) - assert.Equal(t, hostID, cseAct.HostID) - assert.Equal(t, "FailedApp", cseAct.SoftwareTitle) - assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) - - // Second: canceled install software - canceledAct, ok := createdActivities[1].(fleet.ActivityTypeCanceledInstallSoftware) - require.True(t, ok, "expected ActivityTypeCanceledInstallSoftware, got %T", createdActivities[1]) + // Should have 1 activity: canceled install (canceled_setup_experience is now emitted by maybeCancelPendingSetupExperienceSteps) + require.Len(t, createdActivities, 1) + + // Canceled install software + canceledAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledInstallSoftware) + require.True(t, ok, "expected ActivityTypeCanceledInstallSoftware, got %T", createdActivities[0]) assert.Equal(t, hostID, canceledAct.HostID) assert.Equal(t, hostDisplayName, canceledAct.HostDisplayName) assert.Equal(t, "DummyApp", canceledAct.SoftwareTitle) @@ -180,19 +173,12 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { // Status should have been changed to failure assert.Equal(t, fleet.SetupExperienceStatusFailure, results[1].Status) - // Should have 2 activities: canceled setup experience + canceled VPP install - require.Len(t, createdActivities, 2) - - // First: canceled setup experience (emitted in first loop for the failed item) - cseAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledSetupExperience) - require.True(t, ok, "expected ActivityTypeCanceledSetupExperience, got %T", createdActivities[0]) - assert.Equal(t, hostID, cseAct.HostID) - assert.Equal(t, "FailedVPP", cseAct.SoftwareTitle) - assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) + // Should have 1 activity: canceled VPP install (canceled_setup_experience is now emitted by maybeCancelPendingSetupExperienceSteps) + require.Len(t, createdActivities, 1) - // Second: canceled install app store app - canceledAct, ok := createdActivities[1].(fleet.ActivityTypeCanceledInstallAppStoreApp) - require.True(t, ok, "expected ActivityTypeCanceledInstallAppStoreApp, got %T", createdActivities[1]) + // Canceled install app store app + canceledAct, ok := createdActivities[0].(fleet.ActivityTypeCanceledInstallAppStoreApp) + require.True(t, ok, "expected ActivityTypeCanceledInstallAppStoreApp, got %T", createdActivities[0]) assert.Equal(t, hostID, canceledAct.HostID) assert.Equal(t, hostDisplayName, canceledAct.HostDisplayName) assert.Equal(t, "VPPApp", canceledAct.SoftwareTitle) @@ -269,20 +255,15 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { assert.Equal(t, fleet.SetupExperienceStatusPending, results[3].Status) // unchanged assert.Equal(t, fleet.SetupExperienceStatusFailure, results[4].Status) // cancelled -> failed - // Three activities: canceled setup experience, canceled sw install, canceled vpp install - require.Len(t, activities, 3) - - cseAct, ok := activities[0].(fleet.ActivityTypeCanceledSetupExperience) - require.True(t, ok) - assert.Equal(t, "FailedApp", cseAct.SoftwareTitle) - assert.Equal(t, failedTitleID, cseAct.SoftwareTitleID) + // Two activities: canceled sw install + canceled vpp install (canceled_setup_experience is now emitted by maybeCancelPendingSetupExperienceSteps) + require.Len(t, activities, 2) - swAct, ok := activities[1].(fleet.ActivityTypeCanceledInstallSoftware) + swAct, ok := activities[0].(fleet.ActivityTypeCanceledInstallSoftware) require.True(t, ok) assert.Equal(t, "CancelledSW", swAct.SoftwareTitle) assert.True(t, swAct.FromSetupExperience) - vppAct, ok := activities[2].(fleet.ActivityTypeCanceledInstallAppStoreApp) + vppAct, ok := activities[1].(fleet.ActivityTypeCanceledInstallAppStoreApp) require.True(t, ok) assert.Equal(t, "CancelledVPP", vppAct.SoftwareTitle) assert.True(t, vppAct.FromSetupExperience) @@ -328,7 +309,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { require.NoError(t, err) }) - t.Run("no canceled_setup_experience activity when no failed software item", func(t *testing.T) { + t.Run("cancelled items without failed item still emit individual cancel activities", func(t *testing.T) { ds.UpdateSetupExperienceStatusResultFuncInvoked = false installerID := uint(10) @@ -359,7 +340,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { err := svc.recordCanceledSetupExperienceSoftwareActivities(ctx, hostID, hostUUID, hostDisplayName, results) require.NoError(t, err) - // Should only have the canceled install, no canceled_setup_experience + // Should only have the canceled install activity require.Len(t, createdActivities, 1) _, ok := createdActivities[0].(fleet.ActivityTypeCanceledInstallSoftware) require.True(t, ok) diff --git a/ee/server/service/setup_experience.go b/ee/server/service/setup_experience.go index 83c2784ab0d..cecc9600928 100644 --- a/ee/server/service/setup_experience.go +++ b/ee/server/service/setup_experience.go @@ -276,6 +276,12 @@ func (svc *Service) SetupExperienceNextStep(ctx context.Context, host *fleet.Hos svc.logger.WarnContext(ctx, "got an error when attempting to enqueue VPP app install", "err", err, "adam_id", sw.VPPAppAdamID) sw.Status = fleet.SetupExperienceStatusFailure sw.Error = ptr.String(err.Error()) + // Persist the failure before cancelling other steps, so that + // maybeCancelPendingSetupExperienceSteps can find the failed + // item from its loaded statuses. + if err := svc.ds.UpdateSetupExperienceStatusResult(ctx, sw); err != nil { + return false, ctxerr.Wrap(ctx, err, "updating setup experience with vpp install failure") + } failActivity := fleet.ActivityInstalledAppStoreApp{ HostID: host.ID, HostDisplayName: host.DisplayName(), @@ -303,9 +309,9 @@ func (svc *Service) SetupExperienceNextStep(ctx context.Context, host *fleet.Hos } else { sw.NanoCommandUUID = &cmdUUID sw.Status = fleet.SetupExperienceStatusRunning - } - if err := svc.ds.UpdateSetupExperienceStatusResult(ctx, sw); err != nil { - return false, ctxerr.Wrap(ctx, err, "updating setup experience with vpp install command uuid") + if err := svc.ds.UpdateSetupExperienceStatusResult(ctx, sw); err != nil { + return false, ctxerr.Wrap(ctx, err, "updating setup experience with vpp install command uuid") + } } } case softwareRunning == 0 && len(scriptsPending) > 0: diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index cd289f65f4d..55ebff00393 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -3960,7 +3960,7 @@ func (svc *MDMAppleCheckinAndCommandService) CommandAndReportResults(r *mdm.Requ HostUUID: cmdResult.Identifier(), CommandUUID: cmdResult.CommandUUID, CommandStatus: cmdResult.Status, - }, true); err != nil { + }, true, fleet.NewActivityFunc(svc.newActivityFn)); err != nil { return nil, ctxerr.Wrap(r.Context, err, "updating setup experience status from VPP install result") } else if updated { // TODO: call next step of setup experience? diff --git a/server/service/apple_mdm_cmd_results.go b/server/service/apple_mdm_cmd_results.go index 410eeefd637..38ae5481c15 100644 --- a/server/service/apple_mdm_cmd_results.go +++ b/server/service/apple_mdm_cmd_results.go @@ -171,7 +171,7 @@ func NewInstalledApplicationListResultsHandler( HostUUID: installedAppResult.HostUUID(), CommandUUID: expectedInstall.InstallCommandUUID, CommandStatus: terminalStatus, - }, true); err != nil { + }, true, fleet.NewActivityFunc(newActivityFn)); err != nil { return ctxerr.Wrap(ctx, err, "updating setup experience status from VPP install result") } else if updated { fromSetupExperience = true diff --git a/server/service/orbit.go b/server/service/orbit.go index 103ee328beb..0f20f23a932 100644 --- a/server/service/orbit.go +++ b/server/service/orbit.go @@ -855,7 +855,7 @@ func (svc *Service) SaveHostScriptResult(ctx context.Context, result *fleet.Host HostUUID: host.UUID, ExecutionID: result.ExecutionID, ExitCode: result.ExitCode, - }, true); err != nil { + }, true, svc.NewActivity); err != nil { return ctxerr.Wrap(ctx, err, "update setup experience status") } else if updated { svc.logger.DebugContext(ctx, "setup experience script result updated", "host_uuid", host.UUID, "execution_id", result.ExecutionID) @@ -1318,7 +1318,7 @@ func (svc *Service) SaveHostSoftwareInstallResult(ctx context.Context, result *f HostUUID: hostUUID, ExecutionID: result.InstallUUID, InstallerStatus: result.Status(), - }, true); err != nil { + }, true, svc.NewActivity); err != nil { return ctxerr.Wrap(ctx, err, "update setup experience status") } else if updated { svc.logger.DebugContext(ctx, diff --git a/server/service/setup_experience.go b/server/service/setup_experience.go index f4a159f42b0..6ab37e6f7ca 100644 --- a/server/service/setup_experience.go +++ b/server/service/setup_experience.go @@ -266,10 +266,10 @@ func isAllSetupExperienceSoftwareRequired(ctx context.Context, ds fleet.Datastor } func (svc *Service) MaybeCancelPendingSetupExperienceSteps(ctx context.Context, host *fleet.Host) error { - return maybeCancelPendingSetupExperienceSteps(ctx, svc.ds, host) + return maybeCancelPendingSetupExperienceSteps(ctx, svc.ds, host, svc.NewActivity) } -func maybeCancelPendingSetupExperienceSteps(ctx context.Context, ds fleet.Datastore, host *fleet.Host) error { +func maybeCancelPendingSetupExperienceSteps(ctx context.Context, ds fleet.Datastore, host *fleet.Host, newActivityFn fleet.NewActivityFunc) error { // Only macOS and Windows support canceling setup experience steps. if host.Platform != "darwin" && host.Platform != "windows" { return nil @@ -318,6 +318,26 @@ func maybeCancelPendingSetupExperienceSteps(ctx context.Context, ds fleet.Datast if err := ds.CancelPendingSetupExperienceSteps(ctx, hostUUID); err != nil { return ctxerr.Wrap(ctx, err, "cancelling pending setup experience steps") } + + // Emit the canceled_setup_experience activity once at cancellation time. + // Find the software item that failed and triggered this cancellation from the + // already-loaded statuses (no extra DB call). + if newActivityFn != nil { + for _, s := range statuses { + if s.Status == fleet.SetupExperienceStatusFailure && s.IsForSoftware() { + if err := newActivityFn(ctx, nil, fleet.ActivityTypeCanceledSetupExperience{ + HostID: host.ID, + HostDisplayName: host.DisplayName(), + SoftwareTitle: s.Name, + SoftwareTitleID: ptr.ValOrZero(s.SoftwareTitleID), + }); err != nil { + return ctxerr.Wrap(ctx, err, "creating canceled setup experience activity") + } + break + } + } + } + return nil } @@ -328,7 +348,7 @@ func maybeCancelPendingSetupExperienceSteps(ctx context.Context, ds fleet.Datast // supported type, it returns false and an error indicated that the type is not supported. // If the skipPending parameter is true, the datastore will only be updated if the given result // status is not pending. -func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, result interface{}, requireTerminalStatus bool) (bool, error) { +func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, result interface{}, requireTerminalStatus bool, newActivityFn fleet.NewActivityFunc) (bool, error) { var updated bool var err error var status fleet.SetupExperienceStatusResultStatus @@ -377,7 +397,7 @@ func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, r if getHostUUIDErr != nil { return updated, fmt.Errorf("getting host by UUID: %w", getHostUUIDErr) } - cancelErr := maybeCancelPendingSetupExperienceSteps(ctx, ds, host) + cancelErr := maybeCancelPendingSetupExperienceSteps(ctx, ds, host, newActivityFn) if cancelErr != nil { return updated, fmt.Errorf("cancel setup experience after macos software install failure: %w", cancelErr) } diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index 93b7dd0ebd1..e5a08e3e56f 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -305,7 +305,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { vppUUID := "vpp-uuid" t.Run("unsupported result type", func(t *testing.T) { - _, err := maybeUpdateSetupExperienceStatus(ctx, ds, map[string]interface{}{"key": "value"}, true) + _, err := maybeUpdateSetupExperienceStatus(ctx, ds, map[string]interface{}{"key": "value"}, true, nil) require.Error(t, err) require.Contains(t, err.Error(), "unsupported result type") }) @@ -351,7 +351,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { ExecutionID: scriptUUID, ExitCode: tt.exitCode, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, true) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, true, nil) require.NoError(t, err) require.Equal(t, tt.alwaysUpdated, updated) require.Equal(t, tt.alwaysUpdated, ds.MaybeUpdateSetupExperienceScriptStatusFuncInvoked) @@ -409,7 +409,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { ExecutionID: softwareUUID, InstallerStatus: tt.status, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, nil) require.NoError(t, err) require.Equal(t, tt.alwaysUpdated, updated) require.Equal(t, tt.alwaysUpdated, ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked) @@ -429,7 +429,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { return true, nil } ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked = false - updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus) + updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, nil) require.NoError(t, err) shouldUpdate := tt.alwaysUpdated if tt.expectStatus == fleet.SetupExperienceStatusPending || tt.expectStatus == fleet.SetupExperienceStatusRunning { @@ -496,7 +496,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { CommandUUID: vppUUID, CommandStatus: tt.status, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, nil) require.NoError(t, err) require.Equal(t, tt.alwaysUpdated, updated) require.Equal(t, tt.alwaysUpdated, ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked) @@ -517,7 +517,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { } ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked = false - updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus) + updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, nil) require.NoError(t, err) shouldUpdate := tt.alwaysUpdated if tt.expected == fleet.SetupExperienceStatusPending || tt.expected == fleet.SetupExperienceStatusRunning { From 6e857d519b51280b01a13a85c0fd088c5864f064 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 23:23:41 -0500 Subject: [PATCH 12/41] Remove references to previous behavior in comments since these are new tests anyway --- ee/server/service/orbit_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/server/service/orbit_test.go b/ee/server/service/orbit_test.go index 43fc54201b8..54eb709a360 100644 --- a/ee/server/service/orbit_test.go +++ b/ee/server/service/orbit_test.go @@ -114,7 +114,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { // Update should have been called assert.True(t, ds.UpdateSetupExperienceStatusResultFuncInvoked) - // Should have 1 activity: canceled install (canceled_setup_experience is now emitted by maybeCancelPendingSetupExperienceSteps) + // Should have 1 activity: canceled install (canceled_setup_experience is emitted earlier) require.Len(t, createdActivities, 1) // Canceled install software @@ -173,7 +173,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { // Status should have been changed to failure assert.Equal(t, fleet.SetupExperienceStatusFailure, results[1].Status) - // Should have 1 activity: canceled VPP install (canceled_setup_experience is now emitted by maybeCancelPendingSetupExperienceSteps) + // Should have 1 activity: canceled VPP install (canceled_setup_experience is emitted earlier) require.Len(t, createdActivities, 1) // Canceled install app store app @@ -255,7 +255,7 @@ func TestFailCancelledSetupExperienceInstalls(t *testing.T) { assert.Equal(t, fleet.SetupExperienceStatusPending, results[3].Status) // unchanged assert.Equal(t, fleet.SetupExperienceStatusFailure, results[4].Status) // cancelled -> failed - // Two activities: canceled sw install + canceled vpp install (canceled_setup_experience is now emitted by maybeCancelPendingSetupExperienceSteps) + // Two activities: canceled sw install + canceled vpp install (canceled_setup_experience emitted earlier) require.Len(t, activities, 2) swAct, ok := activities[0].(fleet.ActivityTypeCanceledInstallSoftware) From 1fb3469181b201b26b602c4ac4b17ec57bb9a7a7 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 23:40:29 -0500 Subject: [PATCH 13/41] =?UTF-8?q?=F0=9F=A4=96=20Add=20test=20coverage=20fo?= =?UTF-8?q?r=20setup=20experience=20cancel=20activity=20creation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: In `server/service/setup_experience_test.go`, for calls to `maybeUpdateSetupExperienceStatus`, replace nil 5th params with activity functions when it makes sense to spy on the function calls. If we don't have a test where the new activity function would be invoked, add a test that exercises that path. --- server/service/setup_experience_test.go | 117 +++++++++++++++++++++++- 1 file changed, 113 insertions(+), 4 deletions(-) diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index e5a08e3e56f..7cc99956934 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -409,10 +409,16 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { ExecutionID: softwareUUID, InstallerStatus: tt.status, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, nil) + activityFnCalled := false + activityFn := func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityFnCalled = true + return nil + } + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, activityFn) require.NoError(t, err) require.Equal(t, tt.alwaysUpdated, updated) require.Equal(t, tt.alwaysUpdated, ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked) + require.False(t, activityFnCalled) requireTerminalStatus = false // when this flag is false, we do expect pending status to update @@ -429,7 +435,8 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { return true, nil } ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked = false - updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, nil) + activityFnCalled = false + updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, activityFn) require.NoError(t, err) shouldUpdate := tt.alwaysUpdated if tt.expectStatus == fleet.SetupExperienceStatusPending || tt.expectStatus == fleet.SetupExperienceStatusRunning { @@ -437,6 +444,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { } require.Equal(t, shouldUpdate, updated) require.Equal(t, shouldUpdate, ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked) + require.False(t, activityFnCalled) }) } }) @@ -496,10 +504,16 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { CommandUUID: vppUUID, CommandStatus: tt.status, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, nil) + activityFnCalled := false + activityFn := func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityFnCalled = true + return nil + } + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, activityFn) require.NoError(t, err) require.Equal(t, tt.alwaysUpdated, updated) require.Equal(t, tt.alwaysUpdated, ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked) + require.False(t, activityFnCalled) requireTerminalStatus = false // when this flag is false, we do expect pending status to update @@ -516,8 +530,9 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { return true, nil } ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked = false + activityFnCalled = false - updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, nil) + updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, activityFn) require.NoError(t, err) shouldUpdate := tt.alwaysUpdated if tt.expected == fleet.SetupExperienceStatusPending || tt.expected == fleet.SetupExperienceStatusRunning { @@ -525,7 +540,101 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { } require.Equal(t, shouldUpdate, updated) require.Equal(t, shouldUpdate, ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked) + require.False(t, activityFnCalled) }) } }) + + t.Run("software install failure triggers cancel and activity", func(t *testing.T) { + teamID := uint(1) + failedSoftwareTitleID := uint(42) + failedSoftwareName := "FailedApp" + pendingExecID := "pending-exec-id" + + ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFunc = func(ctx context.Context, hUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { + require.Equal(t, hostUUID, hUUID) + require.Equal(t, softwareUUID, executionID) + require.Equal(t, fleet.SetupExperienceStatusFailure, status) + return true, nil + } + ds.HostByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.Host, error) { + return &fleet.Host{ + ID: 1, + UUID: hostUUID, + Platform: "darwin", + TeamID: &teamID, + }, nil + } + ds.TeamLiteFunc = func(ctx context.Context, tid uint) (*fleet.TeamLite, error) { + require.Equal(t, teamID, tid) + return &fleet.TeamLite{ + ID: teamID, + Config: fleet.TeamConfigLite{ + MDM: fleet.TeamMDM{ + MacOSSetup: fleet.MacOSSetup{ + RequireAllSoftware: true, + }, + }, + }, + }, nil + } + + installerID := uint(10) + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, tID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return []*fleet.SetupExperienceStatusResult{ + { + ID: 1, + HostUUID: hostUUID, + Name: failedSoftwareName, + Status: fleet.SetupExperienceStatusFailure, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: &softwareUUID, + SoftwareTitleID: &failedSoftwareTitleID, + }, + { + ID: 2, + HostUUID: hostUUID, + Name: "PendingApp", + Status: fleet.SetupExperienceStatusPending, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: &pendingExecID, + }, + }, nil + } + ds.CancelHostUpcomingActivityFunc = func(ctx context.Context, hID uint, executionID string) (fleet.ActivityDetails, error) { + require.Equal(t, uint(1), hID) + require.Equal(t, pendingExecID, executionID) + return nil, nil + } + ds.CancelPendingSetupExperienceStepsFunc = func(ctx context.Context, hUUID string) error { + require.Equal(t, hostUUID, hUUID) + return nil + } + + var activityFnCalled bool + var recordedActivity fleet.ActivityDetails + activityFn := func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityFnCalled = true + recordedActivity = activity + return nil + } + + result := fleet.SetupExperienceSoftwareInstallResult{ + HostUUID: hostUUID, + ExecutionID: softwareUUID, + InstallerStatus: fleet.SoftwareInstallFailed, + } + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, true, activityFn) + require.NoError(t, err) + require.True(t, updated) + require.True(t, activityFnCalled) + require.True(t, ds.CancelPendingSetupExperienceStepsFuncInvoked) + require.True(t, ds.CancelHostUpcomingActivityFuncInvoked) + + canceledActivity, ok := recordedActivity.(fleet.ActivityTypeCanceledSetupExperience) + require.True(t, ok) + require.Equal(t, uint(1), canceledActivity.HostID) + require.Equal(t, failedSoftwareName, canceledActivity.SoftwareTitle) + require.Equal(t, failedSoftwareTitleID, canceledActivity.SoftwareTitleID) + }) } From 23df02d23fb1724e44086740f21341e6a5c1b870 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Sun, 12 Apr 2026 23:56:17 -0500 Subject: [PATCH 14/41] Fix incremental lint issues --- server/service/setup_experience.go | 2 +- server/service/setup_experience_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/service/setup_experience.go b/server/service/setup_experience.go index 6ab37e6f7ca..52c8582a64e 100644 --- a/server/service/setup_experience.go +++ b/server/service/setup_experience.go @@ -348,7 +348,7 @@ func maybeCancelPendingSetupExperienceSteps(ctx context.Context, ds fleet.Datast // supported type, it returns false and an error indicated that the type is not supported. // If the skipPending parameter is true, the datastore will only be updated if the given result // status is not pending. -func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, result interface{}, requireTerminalStatus bool, newActivityFn fleet.NewActivityFunc) (bool, error) { +func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, result any, requireTerminalStatus bool, newActivityFn fleet.NewActivityFunc) (bool, error) { var updated bool var err error var status fleet.SetupExperienceStatusResultStatus diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index 7cc99956934..e38e56f6b74 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -305,7 +305,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { vppUUID := "vpp-uuid" t.Run("unsupported result type", func(t *testing.T) { - _, err := maybeUpdateSetupExperienceStatus(ctx, ds, map[string]interface{}{"key": "value"}, true, nil) + _, err := maybeUpdateSetupExperienceStatus(ctx, ds, map[string]any{"key": "value"}, true, nil) require.Error(t, err) require.Contains(t, err.Error(), "unsupported result type") }) From ffdc483a29f2b0d5628c126f9471f3d04e98ccca Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 09:17:16 -0500 Subject: [PATCH 15/41] Fix stale error message Resolves https://github.com/fleetdm/fleet/pull/43437/changes#r3070952226 --- server/service/setup_experience.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/service/setup_experience.go b/server/service/setup_experience.go index 52c8582a64e..d2abcbc5dd1 100644 --- a/server/service/setup_experience.go +++ b/server/service/setup_experience.go @@ -399,7 +399,7 @@ func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, r } cancelErr := maybeCancelPendingSetupExperienceSteps(ctx, ds, host, newActivityFn) if cancelErr != nil { - return updated, fmt.Errorf("cancel setup experience after macos software install failure: %w", cancelErr) + return updated, fmt.Errorf("cancel setup experience after software install failure: %w", cancelErr) } } return updated, err From 16ab65883924c03e9bd9156713ee79144f0abfb3 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 09:23:14 -0500 Subject: [PATCH 16/41] Tweak error message Resolves https://github.com/fleetdm/fleet/pull/43437#discussion_r3070641454 --- ee/server/service/orbit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/server/service/orbit.go b/ee/server/service/orbit.go index 3abe1d34887..46fa0c80389 100644 --- a/ee/server/service/orbit.go +++ b/ee/server/service/orbit.go @@ -243,7 +243,7 @@ func (svc *Service) recordCanceledSetupExperienceSoftwareActivities( svc.logger.InfoContext(ctx, "marking setup experience software as canceled", "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") } if r.IsForSoftwarePackage() { if err := svc.NewActivity(ctx, nil, fleet.ActivityTypeCanceledInstallSoftware{ From efec8cf7c1794e723b1b63b3bb45da63d5ae817a Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 09:24:39 -0500 Subject: [PATCH 17/41] Change another log line --- ee/server/service/orbit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/server/service/orbit.go b/ee/server/service/orbit.go index 46fa0c80389..a30be399e4c 100644 --- a/ee/server/service/orbit.go +++ b/ee/server/service/orbit.go @@ -240,7 +240,7 @@ func (svc *Service) recordCanceledSetupExperienceSoftwareActivities( continue } r.Status = fleet.SetupExperienceStatusFailure - svc.logger.InfoContext(ctx, "marking setup experience software as canceled", "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, "marking canceled setup experience software install as failed") From e563b19bbf6f7c6872f749fadbe8b021bb97a5ff Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 09:26:44 -0500 Subject: [PATCH 18/41] Tweak another error message --- ee/server/service/devices.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/server/service/devices.go b/ee/server/service/devices.go index de84d9a287f..2e7100b615a 100644 --- a/ee/server/service/devices.go +++ b/ee/server/service/devices.go @@ -322,7 +322,7 @@ func (svc *Service) getHostSetupExperienceStatus(ctx context.Context, host *flee // 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 From 00f2c6b21ee059f8acaf595b4b9787403a0a1ed6 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 09:34:25 -0500 Subject: [PATCH 19/41] Clean up changes file Fixes https://github.com/fleetdm/fleet/pull/43437#discussion_r3070641481 and https://github.com/fleetdm/fleet/pull/43437#discussion_r3070653347 --- changes/34288-setup-experience-cancel-activity | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/changes/34288-setup-experience-cancel-activity b/changes/34288-setup-experience-cancel-activity index 59933776529..4f6cc462f3c 100644 --- a/changes/34288-setup-experience-cancel-activity +++ b/changes/34288-setup-experience-cancel-activity @@ -1,3 +1,3 @@ -* Added activity when setup experience is cancelled due to software install failure -* Added cancel activities for each VPP app install skipped due to setup experience cancellation, and switched "failed" activity to "cancelled" for package-based software installs in the same situation -* Added install failure activity when VPP installs fail due to licensing issues during setup experience +- 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 From 7aa229d8c6be33b7f02630e001489931c2a3047b Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 09:55:34 -0500 Subject: [PATCH 20/41] Rename test Resolves https://github.com/fleetdm/fleet/pull/43437#discussion_r3070641492 --- ee/server/service/orbit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/server/service/orbit_test.go b/ee/server/service/orbit_test.go index 54eb709a360..e68d74a60a9 100644 --- a/ee/server/service/orbit_test.go +++ b/ee/server/service/orbit_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestFailCancelledSetupExperienceInstalls(t *testing.T) { +func TestRecordCanceledSetupExperienceSoftwareActivities(t *testing.T) { ctx := context.Background() ds := new(mock.Store) baseSvc := new(svcmock.Service) From 9bb544e3d95f9294a887be2d7cde96e6483c9bda Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 10:04:48 -0500 Subject: [PATCH 21/41] Don't emit activity until setup exp status persist succeeds to avoid dupes if that step doesn't succeed Resolves https://github.com/fleetdm/fleet/pull/43437#pullrequestreview-4096168655 --- server/worker/apple_mdm.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/server/worker/apple_mdm.go b/server/worker/apple_mdm.go index a8a4a9cacf7..41c4aba48da 100644 --- a/server/worker/apple_mdm.go +++ b/server/worker/apple_mdm.go @@ -584,6 +584,7 @@ func (a *AppleMDM) installSetupExperienceVPPAppsOnIosIpadOS(ctx context.Context, cmdUUID, err := a.installSoftwareFromVPP(ctx, host, vppApp, true, opts) + failedBeforeCommandSend := err != nil if err != nil { // if we get an error (e.g. no available licenses) while attempting to enqueue the // install, then we should immediately go to an error state so setup experience @@ -591,21 +592,6 @@ func (a *AppleMDM) installSetupExperienceVPPAppsOnIosIpadOS(ctx context.Context, a.Log.ErrorContext(ctx, "got an error when attempting to enqueue VPP app install", "err", err, "adam_id", app.VPPAppAdamID) app.Status = fleet.SetupExperienceStatusFailure app.Error = ptr.String(err.Error()) - // Emit activity for the VPP app install failure - if a.NewActivityFn != nil { - failActivity := fleet.ActivityInstalledAppStoreApp{ - HostID: host.ID, - HostDisplayName: host.DisplayName(), - SoftwareTitle: app.Name, - AppStoreID: ptr.ValOrZero(app.VPPAppAdamID), - Status: string(fleet.SoftwareInstallFailed), - HostPlatform: host.Platform, - FromSetupExperience: true, - } - if actErr := a.NewActivityFn(ctx, nil, failActivity); actErr != nil { - a.Log.WarnContext(ctx, "failed to create activity for VPP app install failure during setup experience", "err", actErr) - } - } } else { app.NanoCommandUUID = &cmdUUID app.Status = fleet.SetupExperienceStatusRunning @@ -614,6 +600,21 @@ func (a *AppleMDM) installSetupExperienceVPPAppsOnIosIpadOS(ctx context.Context, if err := a.Datastore.UpdateSetupExperienceStatusResult(ctx, app); err != nil { return nil, ctxerr.Wrap(ctx, err, "updating setup experience with vpp install command uuid") } + // Emit activity for the VPP app install failure, if one occurred + if failedBeforeCommandSend && a.NewActivityFn != nil { + failActivity := fleet.ActivityInstalledAppStoreApp{ + HostID: host.ID, + HostDisplayName: host.DisplayName(), + SoftwareTitle: app.Name, + AppStoreID: ptr.ValOrZero(app.VPPAppAdamID), + Status: string(fleet.SoftwareInstallFailed), + HostPlatform: host.Platform, + FromSetupExperience: true, + } + if actErr := a.NewActivityFn(ctx, nil, failActivity); actErr != nil { + a.Log.WarnContext(ctx, "failed to create activity for VPP app install failure during setup experience", "err", actErr) + } + } } } From 3b67efb91188e79aec4817e95c5499578bcea239 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 10:06:51 -0500 Subject: [PATCH 22/41] Fall back to Fleet as actor name in new activity Resolves https://github.com/fleetdm/fleet/pull/43437#discussion_r3070653356 --- .../CanceledSetupExperienceActivityItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx index efcf11826b6..def2098040f 100644 --- a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx +++ b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx @@ -18,7 +18,7 @@ const CanceledSetupExperienceActivityItem = ({ hideShowDetails > <> - {activity.actor_full_name} canceled setup experience on this host + {activity.actor_full_name ?? "Fleet"} canceled setup experience on this host because{" "} {getDisplayedSoftwareName( From 6b31ec302a15f246abb7dd57987eef0ce119d750 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 10:21:34 -0500 Subject: [PATCH 23/41] Fix JS lint --- .../CanceledSetupExperienceActivityItem.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx index def2098040f..75d2d1e40c5 100644 --- a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx +++ b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx @@ -18,8 +18,8 @@ const CanceledSetupExperienceActivityItem = ({ hideShowDetails > <> - {activity.actor_full_name ?? "Fleet"} canceled setup experience on this host - because{" "} + {activity.actor_full_name ?? "Fleet"} canceled setup experience + on this host because{" "} {getDisplayedSoftwareName( activity.details.software_title, From d3666ab564b2eb9fc9c85cd17f84eb3d21d2e864 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 11:19:13 -0500 Subject: [PATCH 24/41] Add `from_setup_experience` field to install/script run activities Resolves https://github.com/fleetdm/fleet/pull/43437#discussion_r3074297760 --- server/fleet/activities.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/fleet/activities.go b/server/fleet/activities.go index 8dd4ae08f70..2c024dac92c 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -879,7 +879,7 @@ type ActivityTypeRanScript struct { Async bool `json:"async"` PolicyID *uint `json:"policy_id"` PolicyName *string `json:"policy_name"` - FromSetupExperience bool `json:"-"` + FromSetupExperience bool `json:"from_setup_experience"` } func (a ActivityTypeRanScript) ActivityName() string { @@ -1088,7 +1088,7 @@ type ActivityTypeInstalledSoftware struct { Source *string `json:"source,omitempty"` PolicyID *uint `json:"policy_id"` PolicyName *string `json:"policy_name"` - FromSetupExperience bool `json:"-"` + FromSetupExperience bool `json:"from_setup_experience"` CommandUUID string `json:"command_uuid,omitempty"` } @@ -1331,7 +1331,7 @@ type ActivityInstalledAppStoreApp struct { PolicyID *uint `json:"policy_id"` PolicyName *string `json:"policy_name"` HostPlatform string `json:"host_platform"` - FromSetupExperience bool `json:"-"` + FromSetupExperience bool `json:"from_setup_experience"` FromAutoUpdate bool `json:"from_auto_update"` } From 159194acc9d92843bb2de933309f159c84a501aa Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 11:27:04 -0500 Subject: [PATCH 25/41] Tweak next upcoming activity handling for app store app installs with no command UUID Resolves https://github.com/fleetdm/fleet/pull/43437#discussion_r3074297749 --- server/fleet/activities.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/fleet/activities.go b/server/fleet/activities.go index 2c024dac92c..3e64fe6e877 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -1348,11 +1348,10 @@ func (a ActivityInstalledAppStoreApp) WasFromAutomation() bool { } func (a ActivityInstalledAppStoreApp) MustActivateNextUpcomingActivity() bool { - // for VPP apps, we only activate the next upcoming activity if the installation - // failed, because if it succeeded (and in this case, it only means the command to - // install succeeded), we only activate the next activity when we verify the - // app is actually installed. - return a.Status != string(SoftwareInstalled) + // For VPP apps, we only activate the next upcoming activity if the installation + // failed; successes are activated on install verification. + // More info on the blank command UUID skip at https://github.com/fleetdm/fleet/pull/43437#discussion_r3074297749 + return a.CommandUUID != "" && a.Status != string(SoftwareInstalled) } func (a ActivityInstalledAppStoreApp) ActivateNextUpcomingActivityArgs() (uint, string) { From 41b9500c0df1b70c7b6b8a56e70384998864acb2 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 11:43:24 -0500 Subject: [PATCH 26/41] Remove reference to nonexistent software display name field in activities Resolves https://github.com/fleetdm/fleet/pull/43437#discussion_r3074297746 --- .../GlobalActivityItem/GlobalActivityItem.tsx | 11 ++++------- .../CanceledSetupExperienceActivityItem.tsx | 11 ++--------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx index 6158f45ee3c..7dbc1f912e8 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx @@ -1514,16 +1514,13 @@ const TAGGED_TEMPLATES = { ); }, canceledSetupExperience: (activity: IActivity) => { - const { - software_title: title, - software_display_name: displayName, - host_display_name: hostName, - } = activity.details || {}; + const { software_title: title, host_display_name: hostName } = + activity.details || {}; return ( <> {" "} - canceled setup experience on {hostName} because{" "} - {getDisplayedSoftwareName(title, displayName)} failed to install. + canceled setup experience on {hostName} because {title}{" "} + failed to install. ); }, diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx index 75d2d1e40c5..7a7748489d4 100644 --- a/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx +++ b/frontend/pages/hosts/details/cards/Activity/ActivityItems/CanceledSetupExperienceActivityItem/CanceledSetupExperienceActivityItem.tsx @@ -1,7 +1,6 @@ import React from "react"; import ActivityItem from "components/ActivityItem"; -import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers"; import { IHostActivityItemComponentProps } from "../../ActivityConfig"; @@ -19,14 +18,8 @@ const CanceledSetupExperienceActivityItem = ({ > <> {activity.actor_full_name ?? "Fleet"} canceled setup experience - on this host because{" "} - - {getDisplayedSoftwareName( - activity.details.software_title, - activity.details.software_display_name - )} - {" "} - failed to install. + on this host because {activity.details.software_title} failed to + install. ); From e48bbd2b4683e25fe55f7d4f5c44bf7549e2e1c0 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 12:11:18 -0500 Subject: [PATCH 27/41] =?UTF-8?q?=F0=9F=A4=96=20Add=20activity=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt (after multiple attempts it wouldn't create the test any other way): Commit `159194acc9d92843bb2de933309f159c84a501aa` should fix the issue in the following execution chain: 1. A VPP install is attempted via setup experience for an iOS/iPadOS host that has other upcoming activities queued. 2. installSoftwareFromVPP returns an error (e.g., no available licenses); no MDM command is sent, so cmdUUID is empty. 3. failActivity := fleet.ActivityInstalledAppStoreApp{..., Status: SoftwareInstallFailed} is constructed with CommandUUID left as "". 4. a.NewActivityFn(ctx, nil, failActivity) is called. 5. The activity service invokes failActivity.MustActivateNextUpcomingActivity() → returns true (Status != SoftwareInstalled, no CommandUUID check). 6. ActivateNextUpcomingActivity(ctx, hostID, "") is called. 7. Inside activateNextUpcomingActivity: the delete WHERE execution_id='' is a no-op (empty-string guard). The function then queries the host's upcoming activities and activates the next one. 8. A second upcoming activity is now in "activated" state even though the first one never completed, breaking the intended sequential ordering. Build out a test that covers this workflow, confirming that the commit does indeed solve this issue. Item 8 in the list needs to be checked for as part of this. --- .../internal/service/new_activity_test.go | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/server/activity/internal/service/new_activity_test.go b/server/activity/internal/service/new_activity_test.go index 765ae790b33..c636b5cb90b 100644 --- a/server/activity/internal/service/new_activity_test.go +++ b/server/activity/internal/service/new_activity_test.go @@ -12,6 +12,7 @@ import ( "github.com/fleetdm/fleet/v4/server/activity" "github.com/fleetdm/fleet/v4/server/activity/api" "github.com/fleetdm/fleet/v4/server/activity/internal/types" + "github.com/fleetdm/fleet/v4/server/fleet" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -394,3 +395,92 @@ func TestNewActivityWebhookDisabled(t *testing.T) { require.NoError(t, err) require.True(t, ds.newActivityCalled) } + +// TestVPPInstallFailureEmptyCommandUUIDDoesNotActivateNext exercises the +// scenario where a VPP install is attempted during setup experience for a +// host that has other upcoming activities queued. If the VPP call fails +// before an MDM command is sent (e.g. no available licenses), the +// CommandUUID is empty. In that case the next upcoming activity must NOT +// be activated, because the current activity was never truly started — +// activating the next one would break the intended sequential ordering. +// +// See commit 159194acc9d92843bb2de933309f159c84a501aa for the fix. +func TestVPPInstallFailureEmptyCommandUUIDDoesNotActivateNext(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + activity fleet.ActivityInstalledAppStoreApp + expectActivate bool + }{ + { + name: "failed VPP install with empty command UUID must not activate next upcoming activity", + activity: fleet.ActivityInstalledAppStoreApp{ + HostID: 42, + HostDisplayName: "ios-host", + SoftwareTitle: "Licensed App", + AppStoreID: "99999", + CommandUUID: "", // no MDM command was sent + Status: string(fleet.SoftwareInstallFailed), + FromSetupExperience: true, + }, + expectActivate: false, + }, + { + name: "failed VPP install with command UUID activates next upcoming activity", + activity: fleet.ActivityInstalledAppStoreApp{ + HostID: 42, + HostDisplayName: "ios-host", + SoftwareTitle: "Licensed App", + AppStoreID: "99999", + CommandUUID: "cmd-uuid-abc", + Status: string(fleet.SoftwareInstallFailed), + FromSetupExperience: true, + }, + expectActivate: true, + }, + { + name: "successful VPP install must not activate next (handled by install verification)", + activity: fleet.ActivityInstalledAppStoreApp{ + HostID: 42, + HostDisplayName: "ios-host", + SoftwareTitle: "Licensed App", + AppStoreID: "99999", + CommandUUID: "cmd-uuid-abc", + Status: string(fleet.SoftwareInstalled), + FromSetupExperience: true, + }, + expectActivate: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ds := &newActivityMockDatastore{} + providers := &newActivityMockProviders{ + mockDataProviders: mockDataProviders{ + mockUserProvider: &mockUserProvider{}, + mockHostProvider: &mockHostProvider{}, + }, + } + svc := newTestService(ds, providers) + + err := svc.NewActivity(t.Context(), nil, tt.activity) + require.NoError(t, err) + + assert.Equal(t, tt.expectActivate, providers.activateCalled, + "ActivateNextUpcomingActivity called = %v, want %v", + providers.activateCalled, tt.expectActivate) + + if tt.expectActivate { + assert.Equal(t, tt.activity.HostID, providers.lastHostID) + assert.Equal(t, tt.activity.CommandUUID, providers.lastCmdUUID) + } + + // The activity itself must always be stored regardless of + // whether the next upcoming activity was activated. + require.True(t, ds.newActivityCalled, "activity should still be stored") + }) + } +} From 573ee28a57cadc53fd29344a610886257a1c7d36 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 12:24:58 -0500 Subject: [PATCH 28/41] =?UTF-8?q?=F0=9F=A4=96=20Fix=20test=20activity=20as?= =?UTF-8?q?sertions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: Fix `TestIntegrationsEnterprise` and `TestIntegrationsMDM` failures. Pretty sure they're all due to `from_setup_experience` being added to activities. --- docs/Contributing/reference/audit-logs.md | 12 +++++++++--- server/service/integration_enterprise_test.go | 9 +++++---- server/service/integration_mdm_test.go | 18 +++++++++--------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/docs/Contributing/reference/audit-logs.md b/docs/Contributing/reference/audit-logs.md index a8c64e420f2..bff70b4de7f 100644 --- a/docs/Contributing/reference/audit-logs.md +++ b/docs/Contributing/reference/audit-logs.md @@ -1037,6 +1037,7 @@ This activity contains the following fields: - "async": Whether the script was executed asynchronously. - "policy_id": ID of the policy whose failure triggered the script run. Null if no associated policy. - "policy_name": Name of the policy whose failure triggered the script run. Null if no associated policy. +- "from_setup_experience": Whether the script was run as part of the setup experience. #### Example @@ -1049,7 +1050,8 @@ This activity contains the following fields: "batch_execution_id": "3274d95a-c140-4b17-b185-fb33c93b84e3", "async": false, "policy_id": 123, - "policy_name": "Ensure photon torpedoes are primed" + "policy_name": "Ensure photon torpedoes are primed", + "from_setup_experience": false } ``` @@ -1420,6 +1422,7 @@ This activity contains the following fields: - "policy_id": ID of the policy whose failure triggered the installation. Null if no associated policy. - "policy_name": Name of the policy whose failure triggered installation. Null if no associated policy. - "command_uuid": ID of the in-house app installation. +- "from_setup_experience": Whether the installation was triggered as part of the setup experience. #### Example @@ -1435,7 +1438,8 @@ This activity contains the following fields: "status": "pending", "source": "pkg_packages", "policy_id": 1337, - "policy_name": "Ensure 1Password is installed and up to date" + "policy_name": "Ensure 1Password is installed and up to date", + "from_setup_experience": false } ``` @@ -1698,6 +1702,7 @@ This activity contains the following fields: - "command_uuid": UUID of the MDM command used to install the app. - "policy_id": ID of the policy whose failure triggered the install. Null if no associated policy. - "policy_name": Name of the policy whose failure triggered the install. Null if no associated policy. +- "from_setup_experience": Whether the app was installed as part of the setup experience. #### Example @@ -1710,7 +1715,8 @@ This activity contains the following fields: "app_store_id": "1234567", "command_uuid": "98765432-1234-1234-1234-1234567890ab", "policy_id": 123, - "policy_name": "[Install Software] Logic Pro" + "policy_name": "[Install Software] Logic Pro", + "from_setup_experience": false } ``` diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 65774710605..02c587f78eb 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -8183,7 +8183,7 @@ func (s *integrationEnterpriseTestSuite) TestRunHostSavedScript() { s.lastActivityMatches( fleet.ActivityTypeRanScript{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": %q, "script_name": %q, "script_execution_id": %q, "async": true, "policy_id": null, "policy_name": null, "batch_execution_id": null}`, + `{"host_id": %d, "host_display_name": %q, "script_name": %q, "script_execution_id": %q, "async": true, "policy_id": null, "policy_name": null, "batch_execution_id": null, "from_setup_experience": false}`, host.ID, host.DisplayName(), savedNoTmScript.Name, scriptResultResp.ExecutionID, ), 0, @@ -15701,7 +15701,7 @@ func (s *integrationEnterpriseTestSuite) TestHostScriptSoftDelete() { s.lastActivityOfTypeMatches( fleet.ActivityTypeRanScript{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": %q, "script_name": "", "script_execution_id": %q, "async": true, "policy_id": null, "policy_name": null, "batch_execution_id": null}`, + `{"host_id": %d, "host_display_name": %q, "script_name": "", "script_execution_id": %q, "async": true, "policy_id": null, "policy_name": null, "batch_execution_id": null, "from_setup_experience": false}`, host.ID, host.DisplayName(), scriptExecID), 0) // create a saved script execution request @@ -15723,7 +15723,7 @@ func (s *integrationEnterpriseTestSuite) TestHostScriptSoftDelete() { http.StatusOK) s.lastActivityOfTypeMatches( fleet.ActivityTypeRanScript{}.ActivityName(), - fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "script_name": "script1.sh", "script_execution_id": %q, "async": true, "policy_id": null, "policy_name": null, "batch_execution_id": null}`, + fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "script_name": "script1.sh", "script_execution_id": %q, "async": true, "policy_id": null, "policy_name": null, "batch_execution_id": null, "from_setup_experience": false}`, host.ID, host.DisplayName(), savedScriptExecID), 0) // get the anonymous script result details @@ -17979,7 +17979,8 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers "status": "installed", "source": "apps", "policy_id": %d, - "policy_name": "%s" + "policy_name": "%s", + "from_setup_experience": false }`, host1Team1.ID, host1Team1.DisplayName(), "DummyApp", "dummy_installer.pkg", host1LastInstall.ExecutionID, policy1Team1.ID, policy1Team1.Name), 0) var activityCount int diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 278e33d4109..399d47c08f1 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -14258,7 +14258,7 @@ func (s *integrationMDMTestSuite) TestVPPApps() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_auto_update": false}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false, "from_auto_update": false}`, mdmHost.ID, mdmHost.DisplayName(), errApp.Name, @@ -14344,7 +14344,7 @@ func (s *integrationMDMTestSuite) TestVPPApps() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_auto_update": false}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false, "from_auto_update": false}`, mdmHost.ID, mdmHost.DisplayName(), macOSApp.Name, @@ -14420,7 +14420,7 @@ func (s *integrationMDMTestSuite) TestVPPApps() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_auto_update": false}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false, "from_auto_update": false}`, mdmHost.ID, mdmHost.DisplayName(), addedApp.Name, @@ -14678,7 +14678,7 @@ func (s *integrationMDMTestSuite) TestVPPApps() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": %v, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_auto_update": false}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": %v, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false, "from_auto_update": false}`, installHost.ID, installHost.DisplayName(), app.Name, @@ -15228,7 +15228,7 @@ func (s *integrationMDMTestSuite) TestVPPAppPolicyAutomation() { s.lastActivityMatchesExtended( fleet.ActivityTypeRanScript{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": %q, "script_name": %q, "script_execution_id": %q, "async": true, "policy_id": %d, "policy_name": "%s", "batch_execution_id": null}`, + `{"host_id": %d, "host_display_name": %q, "script_name": %q, "script_execution_id": %q, "async": true, "policy_id": %d, "policy_name": "%s", "batch_execution_id": null, "from_setup_experience": false}`, mdmHost2.ID, mdmHost2.DisplayName(), savedTmScript.Name, scriptExecID, policy3Team1.ID, policy3Team1.Name, ), 0, @@ -15297,7 +15297,7 @@ func (s *integrationMDMTestSuite) TestVPPAppPolicyAutomation() { s.lastActivityMatchesExtended( fleet.ActivityTypeRanScript{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": %q, "script_name": %q, "script_execution_id": %q, "async": true, "policy_id": %d, "policy_name": "%s", "batch_execution_id": null}`, + `{"host_id": %d, "host_display_name": %q, "script_name": %q, "script_execution_id": %q, "async": true, "policy_id": %d, "policy_name": "%s", "batch_execution_id": null, "from_setup_experience": false}`, mdmHost2.ID, mdmHost2.DisplayName(), savedTmScript.Name, scriptExecID, policy3Team1.ID, policy3Team1.Name, ), 0, @@ -15414,7 +15414,7 @@ func (s *integrationMDMTestSuite) TestVPPAppPolicyAutomation() { s.lastActivityMatchesExtended( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": %v, "policy_id": %d, "policy_name": "%s", "host_platform": "%s", "from_auto_update": false}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": %v, "policy_id": %d, "policy_name": "%s", "host_platform": "%s", "from_setup_experience": false, "from_auto_update": false}`, mdmHost.ID, mdmHost.DisplayName(), macOSApp.Name, @@ -15466,7 +15466,7 @@ func (s *integrationMDMTestSuite) TestVPPAppPolicyAutomation() { s.lastActivityMatchesExtended( fleet.ActivityTypeRanScript{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": %q, "script_name": %q, "script_execution_id": %q, "async": true, "policy_id": %d, "policy_name": "%s", "batch_execution_id": null}`, + `{"host_id": %d, "host_display_name": %q, "script_name": %q, "script_execution_id": %q, "async": true, "policy_id": %d, "policy_name": "%s", "batch_execution_id": null, "from_setup_experience": false}`, mdmHost2.ID, mdmHost2.DisplayName(), savedTmScript.Name, scriptExecID, policy3Team1.ID, policy3Team1.Name, ), 0, @@ -15532,7 +15532,7 @@ func (s *integrationMDMTestSuite) TestVPPAppPolicyAutomation() { s.lastActivityMatchesExtended( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "host_platform": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": %v, "policy_id": %d, "policy_name": "%s", "from_auto_update": false}`, + `{"host_id": %d, "host_display_name": "%s", "host_platform": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": %v, "policy_id": %d, "policy_name": "%s", "from_setup_experience": false, "from_auto_update": false}`, mdmHost2.ID, mdmHost2.DisplayName(), mdmHost2.Platform, From 0d9f3d35d9bc9739d765bc7a6311eaed7c4aa7a5 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 12:28:04 -0500 Subject: [PATCH 29/41] Revert audit-logs.md changes from setup experience commit (moving to docs-v4.84.0) --- docs/Contributing/reference/audit-logs.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/docs/Contributing/reference/audit-logs.md b/docs/Contributing/reference/audit-logs.md index bff70b4de7f..a8c64e420f2 100644 --- a/docs/Contributing/reference/audit-logs.md +++ b/docs/Contributing/reference/audit-logs.md @@ -1037,7 +1037,6 @@ This activity contains the following fields: - "async": Whether the script was executed asynchronously. - "policy_id": ID of the policy whose failure triggered the script run. Null if no associated policy. - "policy_name": Name of the policy whose failure triggered the script run. Null if no associated policy. -- "from_setup_experience": Whether the script was run as part of the setup experience. #### Example @@ -1050,8 +1049,7 @@ This activity contains the following fields: "batch_execution_id": "3274d95a-c140-4b17-b185-fb33c93b84e3", "async": false, "policy_id": 123, - "policy_name": "Ensure photon torpedoes are primed", - "from_setup_experience": false + "policy_name": "Ensure photon torpedoes are primed" } ``` @@ -1422,7 +1420,6 @@ This activity contains the following fields: - "policy_id": ID of the policy whose failure triggered the installation. Null if no associated policy. - "policy_name": Name of the policy whose failure triggered installation. Null if no associated policy. - "command_uuid": ID of the in-house app installation. -- "from_setup_experience": Whether the installation was triggered as part of the setup experience. #### Example @@ -1438,8 +1435,7 @@ This activity contains the following fields: "status": "pending", "source": "pkg_packages", "policy_id": 1337, - "policy_name": "Ensure 1Password is installed and up to date", - "from_setup_experience": false + "policy_name": "Ensure 1Password is installed and up to date" } ``` @@ -1702,7 +1698,6 @@ This activity contains the following fields: - "command_uuid": UUID of the MDM command used to install the app. - "policy_id": ID of the policy whose failure triggered the install. Null if no associated policy. - "policy_name": Name of the policy whose failure triggered the install. Null if no associated policy. -- "from_setup_experience": Whether the app was installed as part of the setup experience. #### Example @@ -1715,8 +1710,7 @@ This activity contains the following fields: "app_store_id": "1234567", "command_uuid": "98765432-1234-1234-1234-1234567890ab", "policy_id": 123, - "policy_name": "[Install Software] Logic Pro", - "from_setup_experience": false + "policy_name": "[Install Software] Logic Pro" } ``` From f3c7552dfbdd138b944dbdab872d11c20015bc17 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 13:09:01 -0500 Subject: [PATCH 30/41] Fix missing from_setup_experience expectation in integration test --- server/service/integration_enterprise_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 02c587f78eb..e9103029d65 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -7968,7 +7968,8 @@ func (s *integrationEnterpriseTestSuite) TestRunBatchScript() { "batch_execution_id": "%s", "script_execution_id": "%s", "script_name": "%s", - "host_display_name": "%s" + "host_display_name": "%s", + "from_setup_experience": false } `, host1.ID, batchRes.BatchExecutionID, orbitRespHost1.Notifications.PendingScriptExecutionIDs[0], script.Name, host1.DisplayName()) require.Len(t, hostPastActivitiesResp.Activities, 1) From cf68927be233cce6467b7cd89a8eae61466367b7 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 13:11:52 -0500 Subject: [PATCH 31/41] =?UTF-8?q?=F0=9F=A4=96=20Move=20activity=20test=20t?= =?UTF-8?q?o=20avoid=20including=20Fleet=20core=20activities=20from=20the?= =?UTF-8?q?=20activites=20module?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: Move `TestVPPInstallFailureEmptyCommandUUIDDoesNotActivateNext` to the main module to avoid the activity module depending on the main module. --- .../internal/service/new_activity_test.go | 90 ------------------- server/fleet/activities_test.go | 82 +++++++++++++++++ 2 files changed, 82 insertions(+), 90 deletions(-) create mode 100644 server/fleet/activities_test.go diff --git a/server/activity/internal/service/new_activity_test.go b/server/activity/internal/service/new_activity_test.go index c636b5cb90b..765ae790b33 100644 --- a/server/activity/internal/service/new_activity_test.go +++ b/server/activity/internal/service/new_activity_test.go @@ -12,7 +12,6 @@ import ( "github.com/fleetdm/fleet/v4/server/activity" "github.com/fleetdm/fleet/v4/server/activity/api" "github.com/fleetdm/fleet/v4/server/activity/internal/types" - "github.com/fleetdm/fleet/v4/server/fleet" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -395,92 +394,3 @@ func TestNewActivityWebhookDisabled(t *testing.T) { require.NoError(t, err) require.True(t, ds.newActivityCalled) } - -// TestVPPInstallFailureEmptyCommandUUIDDoesNotActivateNext exercises the -// scenario where a VPP install is attempted during setup experience for a -// host that has other upcoming activities queued. If the VPP call fails -// before an MDM command is sent (e.g. no available licenses), the -// CommandUUID is empty. In that case the next upcoming activity must NOT -// be activated, because the current activity was never truly started — -// activating the next one would break the intended sequential ordering. -// -// See commit 159194acc9d92843bb2de933309f159c84a501aa for the fix. -func TestVPPInstallFailureEmptyCommandUUIDDoesNotActivateNext(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - activity fleet.ActivityInstalledAppStoreApp - expectActivate bool - }{ - { - name: "failed VPP install with empty command UUID must not activate next upcoming activity", - activity: fleet.ActivityInstalledAppStoreApp{ - HostID: 42, - HostDisplayName: "ios-host", - SoftwareTitle: "Licensed App", - AppStoreID: "99999", - CommandUUID: "", // no MDM command was sent - Status: string(fleet.SoftwareInstallFailed), - FromSetupExperience: true, - }, - expectActivate: false, - }, - { - name: "failed VPP install with command UUID activates next upcoming activity", - activity: fleet.ActivityInstalledAppStoreApp{ - HostID: 42, - HostDisplayName: "ios-host", - SoftwareTitle: "Licensed App", - AppStoreID: "99999", - CommandUUID: "cmd-uuid-abc", - Status: string(fleet.SoftwareInstallFailed), - FromSetupExperience: true, - }, - expectActivate: true, - }, - { - name: "successful VPP install must not activate next (handled by install verification)", - activity: fleet.ActivityInstalledAppStoreApp{ - HostID: 42, - HostDisplayName: "ios-host", - SoftwareTitle: "Licensed App", - AppStoreID: "99999", - CommandUUID: "cmd-uuid-abc", - Status: string(fleet.SoftwareInstalled), - FromSetupExperience: true, - }, - expectActivate: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - ds := &newActivityMockDatastore{} - providers := &newActivityMockProviders{ - mockDataProviders: mockDataProviders{ - mockUserProvider: &mockUserProvider{}, - mockHostProvider: &mockHostProvider{}, - }, - } - svc := newTestService(ds, providers) - - err := svc.NewActivity(t.Context(), nil, tt.activity) - require.NoError(t, err) - - assert.Equal(t, tt.expectActivate, providers.activateCalled, - "ActivateNextUpcomingActivity called = %v, want %v", - providers.activateCalled, tt.expectActivate) - - if tt.expectActivate { - assert.Equal(t, tt.activity.HostID, providers.lastHostID) - assert.Equal(t, tt.activity.CommandUUID, providers.lastCmdUUID) - } - - // The activity itself must always be stored regardless of - // whether the next upcoming activity was activated. - require.True(t, ds.newActivityCalled, "activity should still be stored") - }) - } -} diff --git a/server/fleet/activities_test.go b/server/fleet/activities_test.go new file mode 100644 index 00000000000..3bb1052b286 --- /dev/null +++ b/server/fleet/activities_test.go @@ -0,0 +1,82 @@ +package fleet + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestVPPInstallFailureEmptyCommandUUIDDoesNotActivateNext exercises the +// scenario where a VPP install is attempted during setup experience for a +// host that has other upcoming activities queued. If the VPP call fails +// before an MDM command is sent (e.g. no available licenses), the +// CommandUUID is empty. In that case the next upcoming activity must NOT +// be activated, because the current activity was never truly started — +// activating the next one would break the intended sequential ordering. +// +// See commit 159194acc9d92843bb2de933309f159c84a501aa for the fix. +func TestVPPInstallFailureEmptyCommandUUIDDoesNotActivateNext(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + activity ActivityInstalledAppStoreApp + expectActivate bool + }{ + { + name: "failed VPP install with empty command UUID must not activate next upcoming activity", + activity: ActivityInstalledAppStoreApp{ + HostID: 42, + HostDisplayName: "ios-host", + SoftwareTitle: "Licensed App", + AppStoreID: "99999", + CommandUUID: "", // no MDM command was sent + Status: string(SoftwareInstallFailed), + FromSetupExperience: true, + }, + expectActivate: false, + }, + { + name: "failed VPP install with command UUID activates next upcoming activity", + activity: ActivityInstalledAppStoreApp{ + HostID: 42, + HostDisplayName: "ios-host", + SoftwareTitle: "Licensed App", + AppStoreID: "99999", + CommandUUID: "cmd-uuid-abc", + Status: string(SoftwareInstallFailed), + FromSetupExperience: true, + }, + expectActivate: true, + }, + { + name: "successful VPP install must not activate next (handled by install verification)", + activity: ActivityInstalledAppStoreApp{ + HostID: 42, + HostDisplayName: "ios-host", + SoftwareTitle: "Licensed App", + AppStoreID: "99999", + CommandUUID: "cmd-uuid-abc", + Status: string(SoftwareInstalled), + FromSetupExperience: true, + }, + expectActivate: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.expectActivate, tt.activity.MustActivateNextUpcomingActivity(), + "MustActivateNextUpcomingActivity() = %v, want %v", + tt.activity.MustActivateNextUpcomingActivity(), tt.expectActivate) + + if tt.expectActivate { + hostID, cmdUUID := tt.activity.ActivateNextUpcomingActivityArgs() + assert.Equal(t, tt.activity.HostID, hostID) + assert.Equal(t, tt.activity.CommandUUID, cmdUUID) + } + }) + } +} From da46cc604092ee971a95e848f444bda24f9b322a Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 14:14:46 -0500 Subject: [PATCH 32/41] =?UTF-8?q?=F0=9F=A4=96=20Fix=20some=20more=20activi?= =?UTF-8?q?ty=20properties?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: Run and fix failures with `TestIntegrationsMDM`. --- .../integration_mdm_setup_experience_test.go | 13 +++++++----- .../service/integration_vpp_install_test.go | 20 +++++++++---------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/server/service/integration_mdm_setup_experience_test.go b/server/service/integration_mdm_setup_experience_test.go index 08103a7385b..9b61f50f823 100644 --- a/server/service/integration_mdm_setup_experience_test.go +++ b/server/service/integration_mdm_setup_experience_test.go @@ -500,7 +500,8 @@ func (s *integrationMDMTestSuite) TestSetupExperienceFlowWithSoftwareAndScriptAu "status": "installed", "source": "apps", "policy_id": null, - "policy_name": null + "policy_name": null, + "from_setup_experience": true } `, enrolledHost.ID, getHostResp.Host.DisplayName, statusResp.Results.Software[0].Name, getSoftwareTitleResp.SoftwareTitle.SoftwarePackage.Name, installUUID) @@ -580,7 +581,8 @@ func (s *integrationMDMTestSuite) TestSetupExperienceFlowWithSoftwareAndScriptAu "script_name": "%s", "host_display_name": "%s", "script_execution_id": "%s", - "batch_execution_id": null + "batch_execution_id": null, + "from_setup_experience": true } `, enrolledHost.ID, statusResp.Results.Script.Name, getHostResp.Host.DisplayName, execID) @@ -931,7 +933,8 @@ func (s *integrationMDMTestSuite) TestSetupExperienceFlowWithFMAAndVersionRollba "status": "installed", "source": "apps", "policy_id": null, - "policy_name": null + "policy_name": null, + "from_setup_experience": true } `, enrolledHost.ID, getHostResp.Host.DisplayName, titleDetail.SoftwareTitle.SoftwarePackage.Name, installUUID) s.lastActivityMatchesExtended(fleet.ActivityTypeInstalledSoftware{}.ActivityName(), expectedActivityDetail, 0, ptr.Bool(true)) @@ -4146,7 +4149,7 @@ func (s *integrationMDMTestSuite) TestSetupExperienceAndroid() { req := android_service.PubSubPushRequest{PubSubMessage: *reportMsg} s.Do("POST", "/api/v1/fleet/android_enterprise/pubsub", &req, http.StatusOK, "token", string(pubSubToken.Value)) s.lastActivityOfTypeMatches(fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf(`{"app_store_id":%q, - "command_uuid":%q, "host_display_name":%q, "host_id":%d, "host_platform":%q, "policy_id":null, "policy_name":null, "self_service":false, "from_auto_update": false, "software_title":%q, + "command_uuid":%q, "from_auto_update": false, "from_setup_experience": true, "host_display_name":%q, "host_id":%d, "host_platform":%q, "policy_id":null, "policy_name":null, "self_service":false, "software_title":%q, "status":%q}`, app1.AdamID, app1CmdUUID, host.DisplayName(), host.ID, host.Platform, app1.Name, fleet.SoftwareInstalled), 0) // the pending install should now be verified @@ -4366,7 +4369,7 @@ func (s *integrationMDMTestSuite) TestSetupExperienceAndroidCancelOnUnenroll() { {"enrollment_id": null, "host_display_name": %q, "host_serial": %q, "installed_from_dep": false, "platform": %q}`, host1.DisplayName(), "", host1.Platform), 0) // for some reason the serial is force-set to empty string when we create this activity s.lastActivityOfTypeMatches(fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf(`{"app_store_id":%q, - "command_uuid":%q, "host_display_name":%q, "host_id":%d, "host_platform":%q, "policy_id":null, "policy_name":null, "self_service":false, "from_auto_update": false, "software_title":%q, + "command_uuid":%q, "from_auto_update": false, "from_setup_experience": true, "host_display_name":%q, "host_id":%d, "host_platform":%q, "policy_id":null, "policy_name":null, "self_service":false, "software_title":%q, "status":%q}`, app1.AdamID, app1CmdUUID, host1.DisplayName(), host1.ID, host1.Platform, app1.Name, fleet.SoftwareInstallFailed), 0) // host2 and host3 haven't been unenrolled, app install is still pending diff --git a/server/service/integration_vpp_install_test.go b/server/service/integration_vpp_install_test.go index 6465f6f6f18..21b7e226417 100644 --- a/server/service/integration_vpp_install_test.go +++ b/server/service/integration_vpp_install_test.go @@ -435,7 +435,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s"}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false}`, mdmHost.ID, mdmHost.DisplayName(), errApp.Name, @@ -483,7 +483,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s"}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false}`, mdmHost.ID, mdmHost.DisplayName(), addedApp.Name, @@ -600,7 +600,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s"}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false}`, mdmHost.ID, mdmHost.DisplayName(), addedApp.Name, @@ -650,7 +650,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s"}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false}`, mdmHost.ID, mdmHost.DisplayName(), addedApp.Name, @@ -900,7 +900,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s"}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false}`, iosHost.ID, iosHost.DisplayName(), iOSApp.Name, @@ -1008,7 +1008,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s"}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": false, "from_auto_update": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false}`, ipodHost.ID, ipodHost.DisplayName(), iOSApp.Name, @@ -1164,7 +1164,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": true, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_auto_update": false}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": true, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false, "from_auto_update": false}`, data.host.ID, data.host.DisplayName(), data.app.Name, @@ -1847,7 +1847,7 @@ func (s *integrationMDMTestSuite) TestInHouseAppSelfInstall() { // installed activity is now created activityData = fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "command_uuid": %q, "install_uuid": "", "software_title": "ipa_test", "software_package": "", "self_service": true, "status": "installed", - "policy_id": null, "policy_name": null}`, iosHost.ID, iosHost.DisplayName(), installCmdUUID) + "policy_id": null, "policy_name": null, "from_setup_experience": false}`, iosHost.ID, iosHost.DisplayName(), installCmdUUID) s.lastActivityMatches(fleet.ActivityTypeInstalledSoftware{}.ActivityName(), activityData, 0) // host has no more upcoming activities @@ -2179,7 +2179,7 @@ func (s *integrationMDMTestSuite) TestVPPAppScheduledUpdates() { s.lastActivityMatches( fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "from_auto_update": false, "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s"}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "from_auto_update": false, "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false}`, host.ID, host.DisplayName(), "App 1", @@ -2457,7 +2457,7 @@ func (s *integrationMDMTestSuite) TestVPPAppScheduledUpdates() { fleet.ActivityInstalledAppStoreApp{}.ActivityName(), fmt.Sprintf( // See `"from_auto_update": true`. - `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "from_auto_update": true, "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s"}`, + `{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "from_auto_update": true, "status": "%s", "self_service": false, "policy_id": null, "policy_name": null, "host_platform": "%s", "from_setup_experience": false}`, host.ID, host.DisplayName(), "App 1", From 612230e2a643a593fd932b6844ab6ef3674cdea4 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 15:06:33 -0500 Subject: [PATCH 33/41] =?UTF-8?q?=F0=9F=A4=96=20Fix=20edge=20case=20on=20u?= =?UTF-8?q?pdating=20setup=20experience=20status=20result=20statuses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompts in same conversation: > Fix the following code review issue: [https://github.com/fleetdm/fleet/pull/43437#discussion_r3074297752 as markdown] (performed two fixes; one was a bit over-broad) > Pretty sure we _don't_ want to do fix 2, because we still want to emit the cancelled setup experience activity when a single piece of software is queued and its failure cancels setup experience. > Good. Add tests that reproduce the original issue and prove that the guard updates perform the expected fix. --- server/datastore/mysql/setup_experience.go | 12 +- .../datastore/mysql/setup_experience_test.go | 180 +++++++++++++++ server/service/setup_experience_test.go | 209 ++++++++++++++++++ 3 files changed, 395 insertions(+), 6 deletions(-) diff --git a/server/datastore/mysql/setup_experience.go b/server/datastore/mysql/setup_experience.go index a1d20c391ff..5d00dd70ac4 100644 --- a/server/datastore/mysql/setup_experience.go +++ b/server/datastore/mysql/setup_experience.go @@ -862,7 +862,7 @@ WHERE host_uuid = ? func (ds *Datastore) MaybeUpdateSetupExperienceVPPStatus(ctx context.Context, hostUUID string, nanoCommandUUID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { selectStmt := "SELECT id FROM setup_experience_status_results WHERE host_uuid = ? AND nano_command_uuid = ?" - updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ?" + updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ? AND status NOT IN (?, ?, ?)" var id uint if err := ds.writer(ctx).GetContext(ctx, &id, selectStmt, hostUUID, nanoCommandUUID); err != nil { @@ -873,7 +873,7 @@ func (ds *Datastore) MaybeUpdateSetupExperienceVPPStatus(ctx context.Context, ho } return false, err } - res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id) + res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) if err != nil { return false, err } @@ -884,7 +884,7 @@ func (ds *Datastore) MaybeUpdateSetupExperienceVPPStatus(ctx context.Context, ho func (ds *Datastore) MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx context.Context, hostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { selectStmt := "SELECT id FROM setup_experience_status_results WHERE host_uuid = ? AND host_software_installs_execution_id = ?" - updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ?" + updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ? AND status NOT IN (?, ?, ?)" var id uint if err := ds.writer(ctx).GetContext(ctx, &id, selectStmt, hostUUID, executionID); err != nil { @@ -895,7 +895,7 @@ func (ds *Datastore) MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx context } return false, err } - res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id) + res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) if err != nil { return false, err } @@ -906,7 +906,7 @@ func (ds *Datastore) MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx context func (ds *Datastore) MaybeUpdateSetupExperienceScriptStatus(ctx context.Context, hostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { selectStmt := "SELECT id FROM setup_experience_status_results WHERE host_uuid = ? AND script_execution_id = ?" - updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ?" + updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ? AND status NOT IN (?, ?, ?)" var id uint if err := ds.writer(ctx).GetContext(ctx, &id, selectStmt, hostUUID, executionID); err != nil { @@ -917,7 +917,7 @@ func (ds *Datastore) MaybeUpdateSetupExperienceScriptStatus(ctx context.Context, } return false, err } - res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id) + res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) if err != nil { return false, err } diff --git a/server/datastore/mysql/setup_experience_test.go b/server/datastore/mysql/setup_experience_test.go index 41a7a6e0eff..b71941b62f0 100644 --- a/server/datastore/mysql/setup_experience_test.go +++ b/server/datastore/mysql/setup_experience_test.go @@ -34,6 +34,7 @@ func TestSetupExperience(t *testing.T) { {"TestUpdateSetupExperienceScriptWhileEnqueued", testUpdateSetupExperienceScriptWhileEnqueued}, {"TestEnqueueSetupExperienceItemsWindows", testEnqueueSetupExperienceItemsWindows}, {"EnqueueSetupExperienceItemsWithDisplayName", testEnqueueSetupExperienceItemsWithDisplayName}, + {"UpdateStatusGuardsTerminalStates", testUpdateStatusGuardsTerminalStates}, } for _, c := range cases { @@ -1794,6 +1795,185 @@ func testHostInSetupExperience(t *testing.T, ds *Datastore) { require.False(t, inSetupExperience) } +func testUpdateStatusGuardsTerminalStates(t *testing.T, ds *Datastore) { + ctx := context.Background() + hostUUID := uuid.NewString() + + // --- Set up foreign-key references --- + + // User (required for software installer) + user, err := ds.NewUser(ctx, &fleet.User{ + Name: "GuardTest", + Email: "guard@example.com", + GlobalRole: ptr.String("admin"), + Password: []byte("12characterslong!"), + }) + require.NoError(t, err) + + // Software installer + installerID, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + Filename: "guard_test.pkg", + Title: "Guard Test Software", + Version: "1.0.0", + Source: "apps", + Platform: "darwin", + Extension: "pkg", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + }) + require.NoError(t, err) + + // VPP token + app + dataToken, err := test.CreateVPPTokenData(time.Now().Add(24*time.Hour), "Guard Kong", "GuardJungle") + require.NoError(t, err) + tok, err := ds.InsertVPPToken(ctx, dataToken) + require.NoError(t, err) + _, err = ds.UpdateVPPTokenTeams(ctx, tok.ID, []uint{}) + require.NoError(t, err) + vppApp, err := ds.InsertVPPAppWithTeam(ctx, &fleet.VPPApp{ + BundleIdentifier: "com.guard.test", + Name: "guard_test.app", + LatestVersion: "1.0.0", + }, nil) + require.NoError(t, err) + var vppAppsTeamsID uint + err = sqlx.GetContext(ctx, ds.reader(ctx), &vppAppsTeamsID, + `SELECT id FROM vpp_apps_teams WHERE adam_id = ?`, vppApp.AdamID) + require.NoError(t, err) + + // Setup experience script (raw SQL, same pattern as testSetupExperienceStatusResults) + var scriptID uint + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + res, err := q.ExecContext(ctx, `INSERT INTO setup_experience_scripts (name) VALUES (?)`, "guard_test_script") + require.NoError(t, err) + id, err := res.LastInsertId() + require.NoError(t, err) + scriptID = uint(id) //nolint: gosec + return nil + }) + + // --- Helpers --- + + insertRow := func(sesr *fleet.SetupExperienceStatusResult) { + stmt := `INSERT INTO setup_experience_status_results + (id, host_uuid, name, status, software_installer_id, + host_software_installs_execution_id, vpp_app_team_id, + nano_command_uuid, setup_experience_script_id, + script_execution_id, error) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + res, err := q.ExecContext(ctx, stmt, + sesr.ID, sesr.HostUUID, sesr.Name, sesr.Status, + sesr.SoftwareInstallerID, + sesr.HostSoftwareInstallsExecutionID, + sesr.VPPAppTeamID, sesr.NanoCommandUUID, + sesr.SetupExperienceScriptID, + sesr.ScriptExecutionID, sesr.Error) + require.NoError(t, err) + id, err := res.LastInsertId() + require.NoError(t, err) + sesr.ID = uint(id) //nolint: gosec + return nil + }) + } + + readStatus := func(id uint) fleet.SetupExperienceStatusResultStatus { + var status fleet.SetupExperienceStatusResultStatus + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &status, + "SELECT status FROM setup_experience_status_results WHERE id = ?", id) + }) + return status + } + + // --- Negative tests: terminal states must not be overwritten --- + + terminalStatuses := []fleet.SetupExperienceStatusResultStatus{ + fleet.SetupExperienceStatusCancelled, + fleet.SetupExperienceStatusFailure, + fleet.SetupExperienceStatusSuccess, + } + + for _, termStatus := range terminalStatuses { + // Software installer row + execID := uuid.NewString() + row := &fleet.SetupExperienceStatusResult{ + HostUUID: hostUUID, + Name: "sw-" + string(termStatus), + Status: termStatus, + SoftwareInstallerID: ptr.Uint(installerID), + HostSoftwareInstallsExecutionID: ptr.String(execID), + } + insertRow(row) + updated, err := ds.MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx, hostUUID, execID, fleet.SetupExperienceStatusFailure) + require.NoError(t, err) + require.False(t, updated, "software installer row in %s should not be updated", termStatus) + require.Equal(t, termStatus, readStatus(row.ID)) + + // VPP row + nanoUUID := uuid.NewString() + row = &fleet.SetupExperienceStatusResult{ + HostUUID: hostUUID, + Name: "vpp-" + string(termStatus), + Status: termStatus, + VPPAppTeamID: ptr.Uint(vppAppsTeamsID), + NanoCommandUUID: ptr.String(nanoUUID), + } + insertRow(row) + updated, err = ds.MaybeUpdateSetupExperienceVPPStatus(ctx, hostUUID, nanoUUID, fleet.SetupExperienceStatusFailure) + require.NoError(t, err) + require.False(t, updated, "VPP row in %s should not be updated", termStatus) + require.Equal(t, termStatus, readStatus(row.ID)) + + // Script row + scriptExecID := uuid.NewString() + row = &fleet.SetupExperienceStatusResult{ + HostUUID: hostUUID, + Name: "script-" + string(termStatus), + Status: termStatus, + SetupExperienceScriptID: ptr.Uint(scriptID), + ScriptExecutionID: ptr.String(scriptExecID), + } + insertRow(row) + updated, err = ds.MaybeUpdateSetupExperienceScriptStatus(ctx, hostUUID, scriptExecID, fleet.SetupExperienceStatusFailure) + require.NoError(t, err) + require.False(t, updated, "script row in %s should not be updated", termStatus) + require.Equal(t, termStatus, readStatus(row.ID)) + } + + // --- Positive control: pending row CAN be updated --- + + pendingExecID := uuid.NewString() + pendingRow := &fleet.SetupExperienceStatusResult{ + HostUUID: hostUUID, + Name: "sw-pending-positive", + Status: fleet.SetupExperienceStatusPending, + SoftwareInstallerID: ptr.Uint(installerID), + HostSoftwareInstallsExecutionID: ptr.String(pendingExecID), + } + insertRow(pendingRow) + updated, err := ds.MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx, hostUUID, pendingExecID, fleet.SetupExperienceStatusFailure) + require.NoError(t, err) + require.True(t, updated, "pending row should be updated") + require.Equal(t, fleet.SetupExperienceStatusFailure, readStatus(pendingRow.ID)) + + // --- Bug-scenario test: canceled VPP row must not flip to failure --- + + cancelledNanoUUID := uuid.NewString() + cancelledVPPRow := &fleet.SetupExperienceStatusResult{ + HostUUID: hostUUID, + Name: "vpp-canceled-bug", + Status: fleet.SetupExperienceStatusCancelled, + VPPAppTeamID: ptr.Uint(vppAppsTeamsID), + NanoCommandUUID: ptr.String(cancelledNanoUUID), + } + insertRow(cancelledVPPRow) + updated, err = ds.MaybeUpdateSetupExperienceVPPStatus(ctx, hostUUID, cancelledNanoUUID, fleet.SetupExperienceStatusFailure) + require.NoError(t, err) + require.False(t, updated, "cancelled VPP row must not be overwritten by late failure result") + require.Equal(t, fleet.SetupExperienceStatusCancelled, readStatus(cancelledVPPRow.ID)) +} + func testGetSetupExperienceScriptByID(t *testing.T, ds *Datastore) { ctx := context.Background() diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index e38e56f6b74..f25189bef25 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -637,4 +637,213 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { require.Equal(t, failedSoftwareName, canceledActivity.SoftwareTitle) require.Equal(t, failedSoftwareTitleID, canceledActivity.SoftwareTitleID) }) + + t.Run("late arriving result for canceled item does not trigger duplicate activity", func(t *testing.T) { + // This test reproduces the exact scenario from the bug report: + // 1. Software install A fails → triggers cancel of pending VPP install B + emits activity + // 2. Later, B's MDM command result (Error) arrives. The datastore guard returns + // updated=false because B is already in "canceled" state, so the cancel/activity + // path is NOT entered a second time. + + teamID := uint(1) + failedSoftwareTitleID := uint(42) + failedSoftwareName := "FailedApp" + pendingVPPCommandUUID := "pending-vpp-cmd" + installerID := uint(10) + vppTeamID := uint(1) + + // ---- Step 1: Software install A fails ---- + + ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFunc = func(ctx context.Context, hUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { + require.Equal(t, hostUUID, hUUID) + require.Equal(t, softwareUUID, executionID) + require.Equal(t, fleet.SetupExperienceStatusFailure, status) + return true, nil // updated + } + ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked = false + + ds.HostByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.Host, error) { + return &fleet.Host{ + ID: 1, + UUID: hostUUID, + Platform: "darwin", + TeamID: &teamID, + }, nil + } + ds.TeamLiteFunc = func(ctx context.Context, tid uint) (*fleet.TeamLite, error) { + return &fleet.TeamLite{ + ID: teamID, + Config: fleet.TeamConfigLite{ + MDM: fleet.TeamMDM{ + MacOSSetup: fleet.MacOSSetup{ + RequireAllSoftware: true, + }, + }, + }, + }, nil + } + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, tID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return []*fleet.SetupExperienceStatusResult{ + { + ID: 1, + HostUUID: hostUUID, + Name: failedSoftwareName, + Status: fleet.SetupExperienceStatusFailure, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: &softwareUUID, + SoftwareTitleID: &failedSoftwareTitleID, + }, + { + ID: 2, + HostUUID: hostUUID, + Name: "PendingVPPApp", + Status: fleet.SetupExperienceStatusPending, + VPPAppTeamID: &vppTeamID, + NanoCommandUUID: &pendingVPPCommandUUID, + }, + }, nil + } + ds.CancelHostUpcomingActivityFunc = func(ctx context.Context, hID uint, executionID string) (fleet.ActivityDetails, error) { + return nil, nil + } + ds.CancelPendingSetupExperienceStepsFunc = func(ctx context.Context, hUUID string) error { + require.Equal(t, hostUUID, hUUID) + return nil + } + ds.CancelPendingSetupExperienceStepsFuncInvoked = false + ds.CancelHostUpcomingActivityFuncInvoked = false + + activityCallCount := 0 + activityFn := func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityCallCount++ + return nil + } + + result := fleet.SetupExperienceSoftwareInstallResult{ + HostUUID: hostUUID, + ExecutionID: softwareUUID, + InstallerStatus: fleet.SoftwareInstallFailed, + } + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, true, activityFn) + require.NoError(t, err) + require.True(t, updated) + require.True(t, ds.CancelPendingSetupExperienceStepsFuncInvoked) + require.Equal(t, 1, activityCallCount, "activity should have been emitted exactly once") + + // ---- Step 2: Late-arriving VPP result for B (already canceled) ---- + // The datastore guard returns (false, nil) because B's row is already "canceled". + + ds.MaybeUpdateSetupExperienceVPPStatusFunc = func(ctx context.Context, hUUID string, cmdUUID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { + require.Equal(t, hostUUID, hUUID) + require.Equal(t, vppUUID, cmdUUID) + require.Equal(t, fleet.SetupExperienceStatusFailure, status) + return false, nil // guard blocked: row already canceled + } + ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked = false + + // Reset invoked flags so we can assert they are NOT set again. + ds.CancelPendingSetupExperienceStepsFuncInvoked = false + ds.CancelHostUpcomingActivityFuncInvoked = false + + vppResult := fleet.SetupExperienceVPPInstallResult{ + HostUUID: hostUUID, + CommandUUID: vppUUID, + CommandStatus: fleet.MDMAppleStatusError, + } + updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, vppResult, true, activityFn) + require.NoError(t, err) + require.False(t, updated, "update should be blocked by datastore guard") + require.False(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, "cancel should NOT be called again") + require.False(t, ds.CancelHostUpcomingActivityFuncInvoked, "cancel upcoming activity should NOT be called again") + require.Equal(t, 1, activityCallCount, "activity should still have been emitted only once (no duplicate)") + }) + + t.Run("late arriving result for canceled VPP item without guard would have triggered duplicate", func(t *testing.T) { + // This test demonstrates the bug path: if the datastore did NOT have the + // guard (i.e. it returns updated=true for a late-arriving result on an + // already-canceled item), the cancel/activity code would fire again, + // producing a duplicate activity entry. + + teamID := uint(1) + failedSoftwareTitleID := uint(42) + failedSoftwareName := "FailedApp" + installerID := uint(10) + + // Simulate: VPP result arrives and datastore says it WAS updated (no guard). + ds.MaybeUpdateSetupExperienceVPPStatusFunc = func(ctx context.Context, hUUID string, cmdUUID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { + require.Equal(t, hostUUID, hUUID) + require.Equal(t, vppUUID, cmdUUID) + require.Equal(t, fleet.SetupExperienceStatusFailure, status) + return true, nil // no guard: update goes through + } + ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked = false + + ds.HostByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.Host, error) { + return &fleet.Host{ + ID: 1, + UUID: hostUUID, + Platform: "darwin", + TeamID: &teamID, + }, nil + } + ds.TeamLiteFunc = func(ctx context.Context, tid uint) (*fleet.TeamLite, error) { + return &fleet.TeamLite{ + ID: teamID, + Config: fleet.TeamConfigLite{ + MDM: fleet.TeamMDM{ + MacOSSetup: fleet.MacOSSetup{ + RequireAllSoftware: true, + }, + }, + }, + }, nil + } + // Both items are now terminal (A=failure from before, B=failure from the late result). + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, tID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return []*fleet.SetupExperienceStatusResult{ + { + ID: 1, + HostUUID: hostUUID, + Name: failedSoftwareName, + Status: fleet.SetupExperienceStatusFailure, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: &softwareUUID, + SoftwareTitleID: &failedSoftwareTitleID, + }, + { + ID: 2, + HostUUID: hostUUID, + Name: "FailedVPPApp", + Status: fleet.SetupExperienceStatusFailure, + VPPAppTeamID: ptr.Uint(teamID), + }, + }, nil + } + ds.CancelHostUpcomingActivityFunc = func(ctx context.Context, hID uint, executionID string) (fleet.ActivityDetails, error) { + return nil, nil + } + ds.CancelPendingSetupExperienceStepsFunc = func(ctx context.Context, hUUID string) error { + require.Equal(t, hostUUID, hUUID) + return nil + } + ds.CancelPendingSetupExperienceStepsFuncInvoked = false + ds.CancelHostUpcomingActivityFuncInvoked = false + + activityCallCount := 0 + activityFn := func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityCallCount++ + return nil + } + + vppResult := fleet.SetupExperienceVPPInstallResult{ + HostUUID: hostUUID, + CommandUUID: vppUUID, + CommandStatus: fleet.MDMAppleStatusError, + } + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, vppResult, true, activityFn) + require.NoError(t, err) + require.True(t, updated, "without the guard the update goes through") + require.True(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, "cancel IS invoked (this is the duplicate bug)") + require.Equal(t, 1, activityCallCount, "activity IS emitted (this is the duplicate activity)") + }) } From 6bbf54a7a4ef55759a54a5eb32ec2c34a648316a Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 15:11:29 -0500 Subject: [PATCH 34/41] =?UTF-8?q?=F0=9F=A4=96=20Clean=20up=20SELECT=20->?= =?UTF-8?q?=20UPDATEs=20to=20single=20queries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompts: > For the most recent commit, for each of the SQL queries edited, why do we have separate SELECT and UPDATE queries? Given the latter isn't conditioned on the former in any meaningful way, seems like we should just include the WHEREs from the SELECT in the UPDATE. (asked for confirmation) > yes --- server/datastore/mysql/setup_experience.go | 48 +++------------------- 1 file changed, 6 insertions(+), 42 deletions(-) diff --git a/server/datastore/mysql/setup_experience.go b/server/datastore/mysql/setup_experience.go index 5d00dd70ac4..94c203440c8 100644 --- a/server/datastore/mysql/setup_experience.go +++ b/server/datastore/mysql/setup_experience.go @@ -861,68 +861,32 @@ WHERE host_uuid = ? } func (ds *Datastore) MaybeUpdateSetupExperienceVPPStatus(ctx context.Context, hostUUID string, nanoCommandUUID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { - selectStmt := "SELECT id FROM setup_experience_status_results WHERE host_uuid = ? AND nano_command_uuid = ?" - updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ? AND status NOT IN (?, ?, ?)" - - var id uint - if err := ds.writer(ctx).GetContext(ctx, &id, selectStmt, hostUUID, nanoCommandUUID); err != nil { - // TODO: maybe we can use the reader instead for this query - if errors.Is(err, sql.ErrNoRows) { - // return early if no results found - return false, nil - } - return false, err - } - res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) + stmt := `UPDATE setup_experience_status_results SET status = ? WHERE host_uuid = ? AND nano_command_uuid = ? AND status NOT IN (?, ?, ?)` + res, err := ds.writer(ctx).ExecContext(ctx, stmt, status, hostUUID, nanoCommandUUID, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) if err != nil { return false, err } n, _ := res.RowsAffected() - return n > 0, nil } func (ds *Datastore) MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx context.Context, hostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { - selectStmt := "SELECT id FROM setup_experience_status_results WHERE host_uuid = ? AND host_software_installs_execution_id = ?" - updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ? AND status NOT IN (?, ?, ?)" - - var id uint - if err := ds.writer(ctx).GetContext(ctx, &id, selectStmt, hostUUID, executionID); err != nil { - // TODO: maybe we can use the reader instead for this query - if errors.Is(err, sql.ErrNoRows) { - // return early if no results found - return false, nil - } - return false, err - } - res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) + stmt := `UPDATE setup_experience_status_results SET status = ? WHERE host_uuid = ? AND host_software_installs_execution_id = ? AND status NOT IN (?, ?, ?)` + res, err := ds.writer(ctx).ExecContext(ctx, stmt, status, hostUUID, executionID, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) if err != nil { return false, err } n, _ := res.RowsAffected() - return n > 0, nil } func (ds *Datastore) MaybeUpdateSetupExperienceScriptStatus(ctx context.Context, hostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { - selectStmt := "SELECT id FROM setup_experience_status_results WHERE host_uuid = ? AND script_execution_id = ?" - updateStmt := "UPDATE setup_experience_status_results SET status = ? WHERE id = ? AND status NOT IN (?, ?, ?)" - - var id uint - if err := ds.writer(ctx).GetContext(ctx, &id, selectStmt, hostUUID, executionID); err != nil { - // TODO: maybe we can use the reader instead for this query - if errors.Is(err, sql.ErrNoRows) { - // return early if no results found - return false, nil - } - return false, err - } - res, err := ds.writer(ctx).ExecContext(ctx, updateStmt, status, id, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) + stmt := `UPDATE setup_experience_status_results SET status = ? WHERE host_uuid = ? AND script_execution_id = ? AND status NOT IN (?, ?, ?)` + res, err := ds.writer(ctx).ExecContext(ctx, stmt, status, hostUUID, executionID, fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusFailure, fleet.SetupExperienceStatusCancelled) if err != nil { return false, err } n, _ := res.RowsAffected() - return n > 0, nil } From 268c78d2b0b473cbc90457c8a35a7d3a71617065 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 15:16:27 -0500 Subject: [PATCH 35/41] =?UTF-8?q?=F0=9F=A4=96=20Clean=20up=20dead=20code?= =?UTF-8?q?=20on=20terminal=20state=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.5; prompts: > Are there any non-test usages of maybeUpdateSetupExperienceStatus that set `requireTerminalStatus` to `false`? > Cool. Clean up the logic (and remove no-longer-relevant tests) since we're always requiring terminal status. --- server/service/apple_mdm.go | 2 +- server/service/apple_mdm_cmd_results.go | 2 +- server/service/orbit.go | 4 +- server/service/setup_experience.go | 11 ++-- server/service/setup_experience_test.go | 73 +++---------------------- 5 files changed, 17 insertions(+), 75 deletions(-) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 55ebff00393..44e70ac8919 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -3960,7 +3960,7 @@ func (svc *MDMAppleCheckinAndCommandService) CommandAndReportResults(r *mdm.Requ HostUUID: cmdResult.Identifier(), CommandUUID: cmdResult.CommandUUID, CommandStatus: cmdResult.Status, - }, true, fleet.NewActivityFunc(svc.newActivityFn)); err != nil { + }, fleet.NewActivityFunc(svc.newActivityFn)); err != nil { return nil, ctxerr.Wrap(r.Context, err, "updating setup experience status from VPP install result") } else if updated { // TODO: call next step of setup experience? diff --git a/server/service/apple_mdm_cmd_results.go b/server/service/apple_mdm_cmd_results.go index 38ae5481c15..4136c40577d 100644 --- a/server/service/apple_mdm_cmd_results.go +++ b/server/service/apple_mdm_cmd_results.go @@ -171,7 +171,7 @@ func NewInstalledApplicationListResultsHandler( HostUUID: installedAppResult.HostUUID(), CommandUUID: expectedInstall.InstallCommandUUID, CommandStatus: terminalStatus, - }, true, fleet.NewActivityFunc(newActivityFn)); err != nil { + }, fleet.NewActivityFunc(newActivityFn)); err != nil { return ctxerr.Wrap(ctx, err, "updating setup experience status from VPP install result") } else if updated { fromSetupExperience = true diff --git a/server/service/orbit.go b/server/service/orbit.go index 0f20f23a932..8d9aee1ad7a 100644 --- a/server/service/orbit.go +++ b/server/service/orbit.go @@ -855,7 +855,7 @@ func (svc *Service) SaveHostScriptResult(ctx context.Context, result *fleet.Host HostUUID: host.UUID, ExecutionID: result.ExecutionID, ExitCode: result.ExitCode, - }, true, svc.NewActivity); err != nil { + }, svc.NewActivity); err != nil { return ctxerr.Wrap(ctx, err, "update setup experience status") } else if updated { svc.logger.DebugContext(ctx, "setup experience script result updated", "host_uuid", host.UUID, "execution_id", result.ExecutionID) @@ -1318,7 +1318,7 @@ func (svc *Service) SaveHostSoftwareInstallResult(ctx context.Context, result *f HostUUID: hostUUID, ExecutionID: result.InstallUUID, InstallerStatus: result.Status(), - }, true, svc.NewActivity); err != nil { + }, svc.NewActivity); err != nil { return ctxerr.Wrap(ctx, err, "update setup experience status") } else if updated { svc.logger.DebugContext(ctx, diff --git a/server/service/setup_experience.go b/server/service/setup_experience.go index d2abcbc5dd1..e880bc46eca 100644 --- a/server/service/setup_experience.go +++ b/server/service/setup_experience.go @@ -346,9 +346,8 @@ func maybeCancelPendingSetupExperienceSteps(ctx context.Context, ds fleet.Datast // SetupExperienceSoftwareInstallResult, and SetupExperienceVPPInstallResult), it returns a boolean // indicating whether the datastore was updated and an error if one occurred. If the result is not of a // supported type, it returns false and an error indicated that the type is not supported. -// If the skipPending parameter is true, the datastore will only be updated if the given result -// status is not pending. -func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, result any, requireTerminalStatus bool, newActivityFn fleet.NewActivityFunc) (bool, error) { +// The datastore will only be updated if the given result status is a terminal status. +func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, result any, newActivityFn fleet.NewActivityFunc) (bool, error) { var updated bool var err error var status fleet.SetupExperienceStatusResultStatus @@ -358,7 +357,7 @@ func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, r status = v.SetupExperienceStatus() if !status.IsValid() { return false, fmt.Errorf("invalid status: %s", status) - } else if requireTerminalStatus && !status.IsTerminalStatus() { + } else if !status.IsTerminalStatus() { return false, nil } return ds.MaybeUpdateSetupExperienceScriptStatus(ctx, v.HostUUID, v.ExecutionID, status) @@ -368,7 +367,7 @@ func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, r hostUUID = v.HostUUID if !status.IsValid() { return false, fmt.Errorf("invalid status: %s", status) - } else if requireTerminalStatus && !status.IsTerminalStatus() { + } else if !status.IsTerminalStatus() { return false, nil } updated, err = ds.MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx, v.HostUUID, v.ExecutionID, status) @@ -380,7 +379,7 @@ func maybeUpdateSetupExperienceStatus(ctx context.Context, ds fleet.Datastore, r hostUUID = v.HostUUID if !status.IsValid() { return false, fmt.Errorf("invalid status: %s", status) - } else if requireTerminalStatus && !status.IsTerminalStatus() { + } else if !status.IsTerminalStatus() { return false, nil } updated, err = ds.MaybeUpdateSetupExperienceVPPStatus(ctx, v.HostUUID, v.CommandUUID, status) diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index f25189bef25..184dd1e1ddc 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -305,7 +305,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { vppUUID := "vpp-uuid" t.Run("unsupported result type", func(t *testing.T) { - _, err := maybeUpdateSetupExperienceStatus(ctx, ds, map[string]any{"key": "value"}, true, nil) + _, err := maybeUpdateSetupExperienceStatus(ctx, ds, map[string]any{"key": "value"}, nil) require.Error(t, err) require.Contains(t, err.Error(), "unsupported result type") }) @@ -351,7 +351,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { ExecutionID: scriptUUID, ExitCode: tt.exitCode, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, true, nil) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, nil) require.NoError(t, err) require.Equal(t, tt.alwaysUpdated, updated) require.Equal(t, tt.alwaysUpdated, ds.MaybeUpdateSetupExperienceScriptStatusFuncInvoked) @@ -388,8 +388,6 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - requireTerminalStatus := true // when this flag is true, we don't expect pending status to update - ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFunc = func(ctx context.Context, hostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { require.Equal(t, hostUUID, hostUUID) require.Equal(t, executionID, softwareUUID) @@ -414,37 +412,11 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { activityFnCalled = true return nil } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, activityFn) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, activityFn) require.NoError(t, err) require.Equal(t, tt.alwaysUpdated, updated) require.Equal(t, tt.alwaysUpdated, ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked) require.False(t, activityFnCalled) - - requireTerminalStatus = false // when this flag is false, we do expect pending status to update - - ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFunc = func(ctx context.Context, hostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { - require.Equal(t, hostUUID, hostUUID) - require.Equal(t, executionID, softwareUUID) - require.Equal(t, tt.expectStatus, status) - require.True(t, status.IsValid()) - if status.IsTerminalStatus() { - require.True(t, status == fleet.SetupExperienceStatusSuccess || status == fleet.SetupExperienceStatusFailure) - } else { - require.True(t, status == fleet.SetupExperienceStatusPending || status == fleet.SetupExperienceStatusRunning) - } - return true, nil - } - ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked = false - activityFnCalled = false - updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, activityFn) - require.NoError(t, err) - shouldUpdate := tt.alwaysUpdated - if tt.expectStatus == fleet.SetupExperienceStatusPending || tt.expectStatus == fleet.SetupExperienceStatusRunning { - shouldUpdate = true - } - require.Equal(t, shouldUpdate, updated) - require.Equal(t, shouldUpdate, ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked) - require.False(t, activityFnCalled) }) } }) @@ -484,8 +456,6 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - requireTerminalStatus := true // when this flag is true, we don't expect pending status to update - ds.MaybeUpdateSetupExperienceVPPStatusFunc = func(ctx context.Context, hostUUID string, cmdUUID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { require.Equal(t, hostUUID, hostUUID) require.Equal(t, cmdUUID, vppUUID) @@ -509,38 +479,11 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { activityFnCalled = true return nil } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, activityFn) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, activityFn) require.NoError(t, err) require.Equal(t, tt.alwaysUpdated, updated) require.Equal(t, tt.alwaysUpdated, ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked) require.False(t, activityFnCalled) - - requireTerminalStatus = false // when this flag is false, we do expect pending status to update - - ds.MaybeUpdateSetupExperienceVPPStatusFunc = func(ctx context.Context, hostUUID string, cmdUUID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { - require.Equal(t, hostUUID, hostUUID) - require.Equal(t, cmdUUID, vppUUID) - require.Equal(t, tt.expected, status) - require.True(t, status.IsValid()) - if status.IsTerminalStatus() { - require.True(t, status == fleet.SetupExperienceStatusSuccess || status == fleet.SetupExperienceStatusFailure) - } else { - require.True(t, status == fleet.SetupExperienceStatusPending || status == fleet.SetupExperienceStatusRunning) - } - return true, nil - } - ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked = false - activityFnCalled = false - - updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, result, requireTerminalStatus, activityFn) - require.NoError(t, err) - shouldUpdate := tt.alwaysUpdated - if tt.expected == fleet.SetupExperienceStatusPending || tt.expected == fleet.SetupExperienceStatusRunning { - shouldUpdate = true - } - require.Equal(t, shouldUpdate, updated) - require.Equal(t, shouldUpdate, ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked) - require.False(t, activityFnCalled) }) } }) @@ -624,7 +567,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { ExecutionID: softwareUUID, InstallerStatus: fleet.SoftwareInstallFailed, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, true, activityFn) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, activityFn) require.NoError(t, err) require.True(t, updated) require.True(t, activityFnCalled) @@ -724,7 +667,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { ExecutionID: softwareUUID, InstallerStatus: fleet.SoftwareInstallFailed, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, true, activityFn) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, activityFn) require.NoError(t, err) require.True(t, updated) require.True(t, ds.CancelPendingSetupExperienceStepsFuncInvoked) @@ -750,7 +693,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { CommandUUID: vppUUID, CommandStatus: fleet.MDMAppleStatusError, } - updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, vppResult, true, activityFn) + updated, err = maybeUpdateSetupExperienceStatus(ctx, ds, vppResult, activityFn) require.NoError(t, err) require.False(t, updated, "update should be blocked by datastore guard") require.False(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, "cancel should NOT be called again") @@ -840,7 +783,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { CommandUUID: vppUUID, CommandStatus: fleet.MDMAppleStatusError, } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, vppResult, true, activityFn) + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, vppResult, activityFn) require.NoError(t, err) require.True(t, updated, "without the guard the update goes through") require.True(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, "cancel IS invoked (this is the duplicate bug)") From 59ddb1b2f0ff21d0e28dec8ada8c9362e634a370 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 15:21:55 -0500 Subject: [PATCH 36/41] Remove unused import --- .../cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx index 7dbc1f912e8..a440d859fd4 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx @@ -16,7 +16,6 @@ import { formatScriptNameForActivityItem, getPerformanceImpactDescription, } from "utilities/helpers"; -import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers"; import ActivityItem from "components/ActivityItem"; import { ShowActivityDetailsHandler } from "components/ActivityItem/ActivityItem"; From 29650fa0f36d908c82f9bbfacc2c6fc2031604e4 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 15:27:45 -0500 Subject: [PATCH 37/41] Update test comment/name --- server/service/setup_experience_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index 184dd1e1ddc..66e0e4a97e0 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -701,11 +701,8 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { require.Equal(t, 1, activityCallCount, "activity should still have been emitted only once (no duplicate)") }) - t.Run("late arriving result for canceled VPP item without guard would have triggered duplicate", func(t *testing.T) { - // This test demonstrates the bug path: if the datastore did NOT have the - // guard (i.e. it returns updated=true for a late-arriving result on an - // already-canceled item), the cancel/activity code would fire again, - // producing a duplicate activity entry. + t.Run("late arriving result for canceled VPP item without guard should not trigger duplicate", func(t *testing.T) { + // See https://github.com/fleetdm/fleet/pull/43437#discussion_r3074297752 teamID := uint(1) failedSoftwareTitleID := uint(42) From ecb0333e4b9b478d2b58a1b190f0020aa6e0c39d Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 16:17:27 -0500 Subject: [PATCH 38/41] Remove simulated bug test --- server/service/setup_experience_test.go | 88 +------------------------ 1 file changed, 1 insertion(+), 87 deletions(-) diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index 66e0e4a97e0..4fd449063e1 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -582,7 +582,7 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { }) t.Run("late arriving result for canceled item does not trigger duplicate activity", func(t *testing.T) { - // This test reproduces the exact scenario from the bug report: + // See https://github.com/fleetdm/fleet/pull/43437#discussion_r3074297752 // 1. Software install A fails → triggers cancel of pending VPP install B + emits activity // 2. Later, B's MDM command result (Error) arrives. The datastore guard returns // updated=false because B is already in "canceled" state, so the cancel/activity @@ -700,90 +700,4 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { require.False(t, ds.CancelHostUpcomingActivityFuncInvoked, "cancel upcoming activity should NOT be called again") require.Equal(t, 1, activityCallCount, "activity should still have been emitted only once (no duplicate)") }) - - t.Run("late arriving result for canceled VPP item without guard should not trigger duplicate", func(t *testing.T) { - // See https://github.com/fleetdm/fleet/pull/43437#discussion_r3074297752 - - teamID := uint(1) - failedSoftwareTitleID := uint(42) - failedSoftwareName := "FailedApp" - installerID := uint(10) - - // Simulate: VPP result arrives and datastore says it WAS updated (no guard). - ds.MaybeUpdateSetupExperienceVPPStatusFunc = func(ctx context.Context, hUUID string, cmdUUID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { - require.Equal(t, hostUUID, hUUID) - require.Equal(t, vppUUID, cmdUUID) - require.Equal(t, fleet.SetupExperienceStatusFailure, status) - return true, nil // no guard: update goes through - } - ds.MaybeUpdateSetupExperienceVPPStatusFuncInvoked = false - - ds.HostByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.Host, error) { - return &fleet.Host{ - ID: 1, - UUID: hostUUID, - Platform: "darwin", - TeamID: &teamID, - }, nil - } - ds.TeamLiteFunc = func(ctx context.Context, tid uint) (*fleet.TeamLite, error) { - return &fleet.TeamLite{ - ID: teamID, - Config: fleet.TeamConfigLite{ - MDM: fleet.TeamMDM{ - MacOSSetup: fleet.MacOSSetup{ - RequireAllSoftware: true, - }, - }, - }, - }, nil - } - // Both items are now terminal (A=failure from before, B=failure from the late result). - ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, tID uint) ([]*fleet.SetupExperienceStatusResult, error) { - return []*fleet.SetupExperienceStatusResult{ - { - ID: 1, - HostUUID: hostUUID, - Name: failedSoftwareName, - Status: fleet.SetupExperienceStatusFailure, - SoftwareInstallerID: &installerID, - HostSoftwareInstallsExecutionID: &softwareUUID, - SoftwareTitleID: &failedSoftwareTitleID, - }, - { - ID: 2, - HostUUID: hostUUID, - Name: "FailedVPPApp", - Status: fleet.SetupExperienceStatusFailure, - VPPAppTeamID: ptr.Uint(teamID), - }, - }, nil - } - ds.CancelHostUpcomingActivityFunc = func(ctx context.Context, hID uint, executionID string) (fleet.ActivityDetails, error) { - return nil, nil - } - ds.CancelPendingSetupExperienceStepsFunc = func(ctx context.Context, hUUID string) error { - require.Equal(t, hostUUID, hUUID) - return nil - } - ds.CancelPendingSetupExperienceStepsFuncInvoked = false - ds.CancelHostUpcomingActivityFuncInvoked = false - - activityCallCount := 0 - activityFn := func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { - activityCallCount++ - return nil - } - - vppResult := fleet.SetupExperienceVPPInstallResult{ - HostUUID: hostUUID, - CommandUUID: vppUUID, - CommandStatus: fleet.MDMAppleStatusError, - } - updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, vppResult, activityFn) - require.NoError(t, err) - require.True(t, updated, "without the guard the update goes through") - require.True(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, "cancel IS invoked (this is the duplicate bug)") - require.Equal(t, 1, activityCallCount, "activity IS emitted (this is the duplicate activity)") - }) } From e08a85e785553d73df4032626574a0005f135037 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 16:47:48 -0500 Subject: [PATCH 39/41] Fix host-only activity flag added back due to merge conflict --- server/fleet/activities.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/fleet/activities.go b/server/fleet/activities.go index 3e64fe6e877..f8c5909014e 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -1877,10 +1877,6 @@ func (a ActivityTypeClearedPasscode) HostIDs() []uint { return []uint{a.HostID} } -func (a ActivityTypeClearedPasscode) HostOnly() bool { - return true -} - type ActivityTypeCanceledSetupExperience struct { HostID uint `json:"host_id"` HostDisplayName string `json:"host_display_name"` From f0dfef9bc7b4d49eabbfb9a086a994803625b6d8 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 16:58:13 -0500 Subject: [PATCH 40/41] Modernize pointers --- .../datastore/mysql/setup_experience_test.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/server/datastore/mysql/setup_experience_test.go b/server/datastore/mysql/setup_experience_test.go index b71941b62f0..88e9aaceaa5 100644 --- a/server/datastore/mysql/setup_experience_test.go +++ b/server/datastore/mysql/setup_experience_test.go @@ -1805,7 +1805,7 @@ func testUpdateStatusGuardsTerminalStates(t *testing.T, ds *Datastore) { user, err := ds.NewUser(ctx, &fleet.User{ Name: "GuardTest", Email: "guard@example.com", - GlobalRole: ptr.String("admin"), + GlobalRole: new("admin"), Password: []byte("12characterslong!"), }) require.NoError(t, err) @@ -1901,8 +1901,8 @@ func testUpdateStatusGuardsTerminalStates(t *testing.T, ds *Datastore) { HostUUID: hostUUID, Name: "sw-" + string(termStatus), Status: termStatus, - SoftwareInstallerID: ptr.Uint(installerID), - HostSoftwareInstallsExecutionID: ptr.String(execID), + SoftwareInstallerID: new(installerID), + HostSoftwareInstallsExecutionID: new(execID), } insertRow(row) updated, err := ds.MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx, hostUUID, execID, fleet.SetupExperienceStatusFailure) @@ -1916,8 +1916,8 @@ func testUpdateStatusGuardsTerminalStates(t *testing.T, ds *Datastore) { HostUUID: hostUUID, Name: "vpp-" + string(termStatus), Status: termStatus, - VPPAppTeamID: ptr.Uint(vppAppsTeamsID), - NanoCommandUUID: ptr.String(nanoUUID), + VPPAppTeamID: new(vppAppsTeamsID), + NanoCommandUUID: new(nanoUUID), } insertRow(row) updated, err = ds.MaybeUpdateSetupExperienceVPPStatus(ctx, hostUUID, nanoUUID, fleet.SetupExperienceStatusFailure) @@ -1931,8 +1931,8 @@ func testUpdateStatusGuardsTerminalStates(t *testing.T, ds *Datastore) { HostUUID: hostUUID, Name: "script-" + string(termStatus), Status: termStatus, - SetupExperienceScriptID: ptr.Uint(scriptID), - ScriptExecutionID: ptr.String(scriptExecID), + SetupExperienceScriptID: new(scriptID), + ScriptExecutionID: new(scriptExecID), } insertRow(row) updated, err = ds.MaybeUpdateSetupExperienceScriptStatus(ctx, hostUUID, scriptExecID, fleet.SetupExperienceStatusFailure) @@ -1948,8 +1948,8 @@ func testUpdateStatusGuardsTerminalStates(t *testing.T, ds *Datastore) { HostUUID: hostUUID, Name: "sw-pending-positive", Status: fleet.SetupExperienceStatusPending, - SoftwareInstallerID: ptr.Uint(installerID), - HostSoftwareInstallsExecutionID: ptr.String(pendingExecID), + SoftwareInstallerID: new(installerID), + HostSoftwareInstallsExecutionID: new(pendingExecID), } insertRow(pendingRow) updated, err := ds.MaybeUpdateSetupExperienceSoftwareInstallStatus(ctx, hostUUID, pendingExecID, fleet.SetupExperienceStatusFailure) @@ -1964,8 +1964,8 @@ func testUpdateStatusGuardsTerminalStates(t *testing.T, ds *Datastore) { HostUUID: hostUUID, Name: "vpp-canceled-bug", Status: fleet.SetupExperienceStatusCancelled, - VPPAppTeamID: ptr.Uint(vppAppsTeamsID), - NanoCommandUUID: ptr.String(cancelledNanoUUID), + VPPAppTeamID: new(vppAppsTeamsID), + NanoCommandUUID: new(cancelledNanoUUID), } insertRow(cancelledVPPRow) updated, err = ds.MaybeUpdateSetupExperienceVPPStatus(ctx, hostUUID, cancelledNanoUUID, fleet.SetupExperienceStatusFailure) From 13ce6c65902a70487e3484bc3c9266753714971f Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 13 Apr 2026 18:35:44 -0500 Subject: [PATCH 41/41] Add missing "during setup experience" activity item strings Resolves https://github.com/fleetdm/fleet/pull/43437#discussion_r3076277798, https://github.com/fleetdm/fleet/pull/43437#discussion_r3076277802 --- .../GlobalActivityItem/GlobalActivityItem.tsx | 6 ++++-- .../InstalledSoftwareActivityItem.tsx | 11 +++++++++-- .../RanScriptActivityItem/RanScriptActivityItem.tsx | 6 +++++- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx index a440d859fd4..fd55265752b 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx @@ -929,12 +929,14 @@ const TAGGED_TEMPLATES = { ); }, ranScript: (activity: IActivity) => { - const { script_name, host_display_name } = activity.details || {}; + const { script_name, host_display_name, from_setup_experience } = + activity.details || {}; return ( <> {" "} ran {formatScriptNameForActivityItem(script_name)} on{" "} - {host_display_name}. + {host_display_name} + {from_setup_experience ? " during setup experience" : ""}. ); }, diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityItems/InstalledSoftwareActivityItem/InstalledSoftwareActivityItem.tsx b/frontend/pages/hosts/details/cards/Activity/ActivityItems/InstalledSoftwareActivityItem/InstalledSoftwareActivityItem.tsx index 12a5eabaced..c36067b57f6 100644 --- a/frontend/pages/hosts/details/cards/Activity/ActivityItems/InstalledSoftwareActivityItem/InstalledSoftwareActivityItem.tsx +++ b/frontend/pages/hosts/details/cards/Activity/ActivityItems/InstalledSoftwareActivityItem/InstalledSoftwareActivityItem.tsx @@ -20,7 +20,12 @@ const InstalledSoftwareActivityItem = ({ isSoloActivity, }: IHostActivityItemComponentPropsWithShowDetails) => { const { actor_full_name: actorName, details } = activity; - const { self_service, software_title: title, source } = details; + const { + self_service, + software_title: title, + source, + from_setup_experience, + } = details; const status = details.status === "failed" ? "failed_uninstall" : details.status; const isScriptPackageSource = SCRIPT_PACKAGE_SOURCES.includes(source || ""); @@ -50,7 +55,9 @@ const InstalledSoftwareActivityItem = ({ isSoloActivity={isSoloActivity} > <>{actorDisplayName} {installedSoftwarePrefix} {title} on this - host{self_service && " (self-service)"}.{" "} + host + {from_setup_experience ? " during setup experience" : ""} + {self_service && " (self-service)"}. ); }; diff --git a/frontend/pages/hosts/details/cards/Activity/ActivityItems/RanScriptActivityItem/RanScriptActivityItem.tsx b/frontend/pages/hosts/details/cards/Activity/ActivityItems/RanScriptActivityItem/RanScriptActivityItem.tsx index df7ec7bb49c..43d1c77c977 100644 --- a/frontend/pages/hosts/details/cards/Activity/ActivityItems/RanScriptActivityItem/RanScriptActivityItem.tsx +++ b/frontend/pages/hosts/details/cards/Activity/ActivityItems/RanScriptActivityItem/RanScriptActivityItem.tsx @@ -34,7 +34,11 @@ const RanScriptActivityItem = ({ {" "} {ranScriptPrefix}{" "} {formatScriptNameForActivityItem(activity.details?.script_name)} on this - host.{" "} + host + {activity.details?.from_setup_experience + ? " during setup experience" + : ""} + . );