Add backend changes for continuous automations on policies - #45999
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request adds a 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/service/integration_enterprise_test.go (1)
30718-30728: ⚡ Quick winAdd a GitOps apply omission-case assertion for this field.
This test validates explicit
true/falseupdates, but not the behavior whenContinuousAutomationsEnabledis omitted from a subsequent/spec/policiesapply payload. Adding that case would lock down the contract for YAMLs that don’t specify the field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/service/integration_enterprise_test.go` around lines 30718 - 30728, Add a test case that verifies omission of ContinuousAutomationsEnabled does not change the stored value: after the existing s.Do POST that sets ContinuousAutomationsEnabled explicitly, call s.Do POST to "/api/latest/fleet/spec/policies" with an ApplyPolicySpecsRequest containing a PolicySpec that omits the ContinuousAutomationsEnabled field, then fetch the persisted policy (via the existing list/get helper used elsewhere in the test) and assert that the policy's ContinuousAutomationsEnabled value remains the same as before; reference the PolicySpec struct, the ApplyPolicySpecsRequest payload and the ContinuousAutomationsEnabled field when locating where to add this omission-case assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/service/integration_enterprise_test.go`:
- Line 30842: The single equality assertion that uses require.Equal(t, 1,
countExecutionsFor(transitionPolicy.ID)) can be flaky because async enqueues may
occur after the one-time check; replace it with a short polling assertion that
repeatedly samples countExecutionsFor(transitionPolicy.ID) over a small time
window (e.g., poll every 50–200ms for ~500–1000ms) and fail if the count changes
from the expected value (1) during that window; implement this inline in the
test or add a small helper (e.g., assertNoNewExecutions(t, transitionPolicy.ID,
expectedCount, timeout)) and use it at the three locations instead of the single
require.Equal call.
---
Nitpick comments:
In `@server/service/integration_enterprise_test.go`:
- Around line 30718-30728: Add a test case that verifies omission of
ContinuousAutomationsEnabled does not change the stored value: after the
existing s.Do POST that sets ContinuousAutomationsEnabled explicitly, call s.Do
POST to "/api/latest/fleet/spec/policies" with an ApplyPolicySpecsRequest
containing a PolicySpec that omits the ContinuousAutomationsEnabled field, then
fetch the persisted policy (via the existing list/get helper used elsewhere in
the test) and assert that the policy's ContinuousAutomationsEnabled value
remains the same as before; reference the PolicySpec struct, the
ApplyPolicySpecsRequest payload and the ContinuousAutomationsEnabled field when
locating where to add this omission-case assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 89a74f8b-b7c9-4b8d-a2ee-682e39319b08
📒 Files selected for processing (10)
changes/42651-continuous-policy-automationscmd/fleetctl/fleetctl/generate_gitops.goserver/datastore/mysql/migrations/tables/20260521141405_AddContinuousAutomationsEnabledToPolicies.goserver/datastore/mysql/migrations/tables/20260521141405_AddContinuousAutomationsEnabledToPolicies_test.goserver/datastore/mysql/policies.goserver/fleet/api_policies.goserver/fleet/policies.goserver/service/integration_enterprise_test.goserver/service/osquery.goserver/service/team_policies.go
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for “continuous” policy automations for team policies, allowing attached software installs / VPP installs / scripts to trigger on every failing policy result (not only pass→fail transitions). This spans the policy API surface, persistence, osquery ingest behavior, GitOps, and integration coverage.
Changes:
- Added
continuous_automations_enabledto policy types/requests/specs and wired it through team policy create/modify flows. - Persisted the new flag in MySQL (migration + updates/inserts) and threaded it into automation processing on incoming failing policy results.
- Updated
fleetctl generate-gitopsoutput and added enterprise integration tests covering CRUD + behavior.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/team_policies.go | Pass through and apply continuous_automations_enabled on create/modify |
| server/service/osquery.go | Trigger automations on every failing result when continuous mode is enabled |
| server/service/integration_enterprise_test.go | Integration tests for CRUD + continuous automation behavior |
| server/fleet/policies.go | Added the field to payload/data/spec structs and DB-mapped structs |
| server/fleet/api_policies.go | Exposed the field on the team policy create request |
| server/datastore/mysql/policies.go | Added column to selects + writes, threaded through ApplyPolicySpecs and lookups |
| server/datastore/mysql/migrations/tables/20260521141405_AddContinuousAutomationsEnabledToPolicies.go | Migration adding continuous_automations_enabled to policies |
| server/datastore/mysql/migrations/tables/20260521141405_AddContinuousAutomationsEnabledToPolicies_test.go | Migration test validating default and explicit values |
| cmd/fleetctl/fleetctl/generate_gitops.go | Include continuous_automations_enabled in generated GitOps YAML |
| changes/42651-continuous-policy-automations | Changelog entry for the new behavior and GitOps surfacing |
Comments suppressed due to low confidence (2)
server/service/integration_enterprise_test.go:30682
- Repo guidance discourages using the legacy
server/ptrhelpers in new code (see.claude/CLAUDE.md), but this new test code introducesptr.Bool(false). Prefer Go'snew(expression)pointer literals (e.g.,new(false)) for*boolfields.
server/service/integration_enterprise_test.go:30843 - After waiting for the continuous policy to reach 2 executions, the test immediately asserts the transition-only policy is still at 1. Because queuing work is async, a delayed (incorrect) re-queue could happen after this point and the test would still pass. Consider asserting the transition-only count stays at 1 for a duration (e.g.,
assert.Never/assert.Consistentlyover ~1–2s) to make the test robust.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #45999 +/- ##
==========================================
+ Coverage 66.83% 66.84% +0.01%
==========================================
Files 2756 2756
Lines 220200 220777 +577
Branches 10916 10993 +77
==========================================
+ Hits 147171 147584 +413
- Misses 59736 59831 +95
- Partials 13293 13362 +69
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Reject continuous_automations_enabled on "All fleets" (global) policies in both the modify endpoint and ApplyPolicySpecs, matching the existing conditional_access_enabled constraint. - Add a premium license check for continuous_automations_enabled in ApplyPolicySpecs (the GitOps path), since the REST decoder's premium gate doesn't apply there. - Replace ptr.Bool with new(true/false) in the new test code and use require.Never for negative assertions so a delayed enqueue doesn't slip past a single point-in-time check. - Update schema.sql with the new column and the migration's row in migration_status_tables. - Refresh fixtures that compare full policy JSON/YAML (cmd/fleetctl/fleetctl/testdata/*, server/webhooks/failing_policies_test.go, cmd/fleetctl/fleetctl/api_test.go, expectedGlobalPolicies.yaml, expectedTeamPolicies.yaml, test_dir_*/default.yml, test_dir_premium/fleets/team-a-thumbsup.yml) so they include continuous_automations_enabled. - gofmt fix for TeamPolicyRequest field alignment in api_policies.go.
Removes the github.com/fleetdm/fleet/v4/server/ptr import from this file
and uses Go 1.26+ new(expression) syntax everywhere instead. For *uint /
*int64 / *float64 fields backed by untyped integer/float literals, an
explicit type conversion (e.g. new(uint(42))) preserves the original
field type. Double-pointer helpers (ptr.BoolPtr, ptr.StringPtr,
ptr.TimePtr, ptr.Float64Ptr) become new(new(x)).
Also switches the new assertCountStable helper's variadic type from
interface{} to any, fixing the only remaining lint issue from
make lint-go-incremental.
The expected JSON output in TestRunApiCommand/create_policy is compared as a raw string, so the field order must match Go's encoding/json declaration-order serialization. Since ContinuousAutomationsEnabled is declared after Type in PolicyData, continuous_automations_enabled must appear after type in the fixture.
The previous version had two latent bugs that made the test fail: 1) Both policies shared a single script. The processScripts code skips a queue if the script is already pending for the host (IsExecutionPendingForHost), so the second policy's automation was never queued. Give each policy its own script. 2) Only one script can be activated at a time on a host (activateNextUpcomingActivity), so submitting two failing policy results back-to-back left the second script queued but unactivated — it never lands in host_script_results until the first completes. Restructure the test like TestPolicyAutomationsContinuousSoftwareInstaller: submit, wait for the count to record, complete, then move on.
TestGenerateGitops compares the generated unassigned.yml against the fixture; with the new field appearing in the output it also has to appear in the expected fixture.
Final check: after the fail→fail re-trigger sequence, submit a passing result for both policies and verify that neither script is queued — passing results never trigger automations, regardless of continuous_automations_enabled.
nulmete
reviewed
May 26, 2026
nulmete
left a comment
nulmete
left a comment
Member
There was a problem hiding this comment.
LGTM, I just have a minor doubt with VPP app installs. I believe we track the installs on host_vpp_software_installs and I'm wondering if we need to reset the retry_count when the new setting is enabled on a policy.
nulmete
previously approved these changes
May 26, 2026
nulmete
approved these changes
May 26, 2026
This was referenced May 27, 2026
4 tasks
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #45149 and #45150.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Database migrations
Checked schema for all modified table for columns that will auto-update timestamps during migration.
Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
Ensured the correct collation is explicitly set for character columns (
COLLATE utf8mb4_unicode_ci).Verified that the setting is exported via
fleetctl generate-gitopsVerified the setting is documented in a separate PR to the GitOps documentation
Verified that the setting is cleared on the server if it is not supplied in a YAML file (or that it is documented as being optional)
Summary by CodeRabbit
New Features
Tests