Patch when closed: install skip, no-retry, and activity - #49858
Merged
cdcme merged 2 commits intoJul 28, 2026
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/39962-patch-when-closed #49858 +/- ##
===============================================================
Coverage ? 67.91%
===============================================================
Files ? 3891
Lines ? 248619
Branches ? 13022
===============================================================
Hits ? 168847
Misses ? 64553
Partials ? 15219
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jkatz01
reviewed
Jul 24, 2026
| // createFailingPolicy makes a global policy (optionally patch-when-closed) and marks it failing | ||
| // for the host so a retry would be eligible. | ||
| createFailingPolicy := func(t *testing.T, host *fleet.Host, patchWhenClosed bool) uint { | ||
| policy, err := ds.NewGlobalPolicy(ctx, &user.ID, fleet.PolicyPayload{ |
Member
There was a problem hiding this comment.
I don't think a global policy can also have patch_when_closed (meaningfully). Should this test be using team policies instead?
Member
Author
There was a problem hiding this comment.
Yeah, good call! Switched the fixture to a team policy. The direct column set has to stay for now since the create path doesn't accept the flag until the API PR lands, but it's a valid team policy now. 👍
jkatz01
approved these changes
Jul 28, 2026
3 tasks
cdcme
added a commit
that referenced
this pull request
Aug 14, 2026
**Related issue:** Resolves #51052 Renames the `installed_software` activity detail flag from `install_skipped_when_app_open` to `skipped_install`, the name Product approved and the one `docs-v4.91.0` already publishes (`audit-logs.md:1661`). The implementation (#49858) landed six days before the activity docs (#49106), so the two diverged; since patch-when-closed is unreleased, this renames outright rather than aliasing. The name is deliberately reason-agnostic: #39178 will set the same flag for a second skip reason (notify-then-defer), where `install_skipped_when_app_open` would be false. Rename only — no behavior change. Covers the JSON tag, the Go field (`InstallSkippedWhenAppOpen` → `SkippedInstall`), and the frontend, where all four readers were swept: activity feed, host activity card, policy automations table, and the install-details modal (including its `installSkippedWhenAppOpen` prop). No endpoint paths changed. Deliberately unchanged: `isAppOpenSkip` in `orbit.go` still names the detection of the app-open case specifically, and the pre-existing frontend-only `"skipped_install"` display status (`interfaces/software.ts`) is not an API value — after this rename the two simply agree. No changes file: `~unreleased bug`, and `changes/39962-patch-when-closed` already covers the feature. No migration: activity details are an opaque `activity_past.details` JSON blob and no SQL reads this key. A `renameto:` alias was considered and rejected — aliases are applied before the INSERT, so it would permanently double-write a never-released key into every row. > [!IMPORTANT] > `rc-minor-fleet-v4.91.0` was cut on 2026-08-11 and still carries the old name. This needs a cherry-pick into that branch before the 4.91.0 cut, or 4.91.0 ships `install_skipped_when_app_open` while its own reference docs document `skipped_install`. # Checklist for submitter ## Testing - [x] Added/updated automated tests - [ ] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated installed-software activity handling to consistently identify installations skipped because the app was open. * Preserved skipped-install status messages, labels, icons, and detail visibility across activity views and policy automation tables. * **Refactor** * Standardized the activity field name used for skipped installations across the application. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #49417
Makes an "app was open" result on a patch-when-closed policy install a skip rather than a failure:
SaveHostSoftwareInstallResult— policypatch_when_closed+ empty pre-install output — and labels theinstalled_softwareactivity withinstall_skipped_when_app_open.attempt_number = 0(excluded from the retry-sequence count) and doesn't queue an immediate retry — the next continuous-automation run re-fires.pre_install_queryon a non-managed policy still fails and still counts toward the retry limit.The
changes/entry for this feature lives on the base feature branch per the stacked-PR convention.Checklist for submitter
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements).Testing
software_install/resultendpoint end-to-end against a running server: app-open result →failed_install, activityinstall_skipped_when_app_open: true,attempt_number = 0, no retry queued, and install details showing "The app was open".pre_install_queryon a non-managed policy still fails, counts (attempt_number = 1), and queues a retry.