From 359d2947d89b6f75365f1f65bdb4cb23934c75e5 Mon Sep 17 00:00:00 2001 From: Juan Fernandez Date: Thu, 2 Jul 2026 09:59:59 -0400 Subject: [PATCH] Fix installed_software status in policy automation activities Relates to #38670 The policies/:id/automation_activities endpoint derived the top-level status for installed_software activities from the live host_software_installs.status generated column. That column becomes NULL when the install row is marked removed=1 (e.g. after the installer package is edited/updated or the software is re-installed), so a historically-successful install was miscategorized as "error". Derive the outcome from the activity's recorded details.status instead, which reflects the install result at the time the activity was created. The install output still comes from host_software_installs. This applies to both the displayed status and the ?status=error|success filter. Also fixed alignment with the info icon on the policy automations table. --- .../_styles.scss | 7 +++ server/datastore/mysql/activities.go | 6 +- server/datastore/mysql/activities_test.go | 59 ++++++++++++++++++- 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/frontend/pages/policies/details/components/PolicyAutomationsActivitiesTable/_styles.scss b/frontend/pages/policies/details/components/PolicyAutomationsActivitiesTable/_styles.scss index 53fc2676b8d..8d3994419ce 100644 --- a/frontend/pages/policies/details/components/PolicyAutomationsActivitiesTable/_styles.scss +++ b/frontend/pages/policies/details/components/PolicyAutomationsActivitiesTable/_styles.scss @@ -83,6 +83,13 @@ &:active { background-color: transparent; } + + &.button > .children-wrapper { + display: flex; + align-items: center; + gap: $pad-small; + width: 100%; + } } &__details-text { diff --git a/server/datastore/mysql/activities.go b/server/datastore/mysql/activities.go index fa05a70045b..3fe6cc90531 100644 --- a/server/datastore/mysql/activities.go +++ b/server/datastore/mysql/activities.go @@ -1665,10 +1665,10 @@ var policyAutomationTaskBranches = []policyAutomationTaskBranch{ ON hsi.host_id = ahp.host_id AND hsi.execution_id = ap.details->>'$.install_uuid' AND hsi.policy_id = ?`, - errorCond: "hsi.status = 'failed_install'", - successCond: "hsi.status = 'installed'", + errorCond: "ap.details->>'$.status' = 'failed_install'", + successCond: "ap.details->>'$.status' = 'installed'", statusCols: statusOutputCols{ - status: "IF(hsi.status = 'installed', 'success', 'error')", + status: "IF(ap.details->>'$.status' = 'installed', 'success', 'error')", output: "hsi.install_script_output", }, }, diff --git a/server/datastore/mysql/activities_test.go b/server/datastore/mysql/activities_test.go index 791272e7dc5..73cc4057acd 100644 --- a/server/datastore/mysql/activities_test.go +++ b/server/datastore/mysql/activities_test.go @@ -2599,12 +2599,31 @@ func testListPolicyAutomationActivities(t *testing.T, ds *Datastore) { require.NoError(t, activitySvc.NewActivity(ctx, nil, dummyActivity{ name: "installed_software", - details: map[string]any{"install_uuid": swSuccessExecID, "software_title": "My Software"}, + details: map[string]any{"install_uuid": swSuccessExecID, "software_title": "My Software", "status": "installed"}, hostIDs: []uint{h1.ID}, })) require.NoError(t, activitySvc.NewActivity(ctx, nil, dummyActivity{ name: "installed_software", - details: map[string]any{"install_uuid": swFailureExecID, "software_title": "My Software"}, + details: map[string]any{"install_uuid": swFailureExecID, "software_title": "My Software", "status": "failed_install"}, + hostIDs: []uint{h1.ID}, + })) + + // A historically-successful install whose host_software_installs row was later + // marked removed (e.g. after the installer package was edited or the software + // re-installed). The generated status column is NULL for such rows, so the + // outcome must come from the recorded details.status, not the live column. + swRemovedExecID := "sw-removed-exec-1" + _, err = ds.writer(ctx).ExecContext(ctx, + `INSERT INTO host_software_installs + (host_id, execution_id, software_installer_id, install_script_exit_code, + install_script_output, pre_install_query_output, post_install_script_output, policy_id, removed) + VALUES + (?, ?, 1, 0, 'install ok', 'pre ok', 'post ok', ?, 1)`, + h1.ID, swRemovedExecID, policy.ID) + require.NoError(t, err) + require.NoError(t, activitySvc.NewActivity(ctx, nil, dummyActivity{ + name: "installed_software", + details: map[string]any{"install_uuid": swRemovedExecID, "software_title": "My Software", "status": "installed"}, hostIDs: []uint{h1.ID}, })) @@ -2699,6 +2718,42 @@ func testListPolicyAutomationActivities(t *testing.T, ds *Datastore) { require.Positive(t, types["installed_app_store_app"]) }) + t.Run("removed install row is categorized by recorded status", func(t *testing.T) { + activities, _, err := ds.ListPolicyAutomationActivities(ctx, policy.ID, adminFilter, listOpts(), "") + require.NoError(t, err) + + // hasRemovedInstall reports whether the removed install activity appears in + // the given result set (matched by its install_uuid in the details blob). + hasRemovedInstall := func(as []*fleet.PolicyAutomationActivity) bool { + for _, a := range as { + require.NotNil(t, a.Details) + var m map[string]any + require.NoError(t, json.Unmarshal(*a.Details, &m)) + if uuid, _ := m["install_uuid"].(string); uuid == swRemovedExecID { + // The live host_software_installs.status is NULL (removed=1), but + // the recorded details.status is "installed", so the historical + // outcome must be reported as success. + require.Equal(t, "success", a.Status) + return true + } + } + return false + } + require.True(t, hasRemovedInstall(activities), "expected the removed install activity to be returned") + + // The status filter must agree with the reported status: the removed row + // is a success, so it appears under status=success and not status=error. + // This guards errorCond/successCond, which the unfiltered query above does + // not exercise. + success, _, err := ds.ListPolicyAutomationActivities(ctx, policy.ID, adminFilter, listOpts(), "success") + require.NoError(t, err) + require.True(t, hasRemovedInstall(success), "removed install should appear under status=success") + + errored, _, err := ds.ListPolicyAutomationActivities(ctx, policy.ID, adminFilter, listOpts(), "error") + require.NoError(t, err) + require.False(t, hasRemovedInstall(errored), "removed install must not appear under status=error") + }) + t.Run("status and output are populated per activity", func(t *testing.T) { activities, _, err := ds.ListPolicyAutomationActivities(ctx, policy.ID, adminFilter, listOpts(), "") require.NoError(t, err)