Wipe host cancels all upcoming activities - #44323
Merged
MagnusHJensen merged 15 commits intoMay 1, 2026
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #44323 +/- ##
==========================================
- Coverage 66.79% 66.75% -0.04%
==========================================
Files 2630 2635 +5
Lines 211355 212423 +1068
Branches 9547 9547
==========================================
+ Hits 141170 141813 +643
- Misses 57359 57728 +369
- Partials 12826 12882 +56
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:
|
…batch-cancel activities
…tch-cancel activities
mna
commented
Apr 29, 2026
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a datastore batch operation to cancel all upcoming activities for a host and refactors the internal cancellation flow to suppress intermediate activations during multi-cancel operations. Introduces Possibly related PRs
🚥 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 docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.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
♻️ Duplicate comments (1)
server/service/apple_mdm.go (1)
4111-4118:⚠️ Potential issue | 🟠 MajorDon’t fail
CommandAndReportResultson post-wipe activity cleanup.
BatchCancelAllHostUpcomingActivitiesaborts the whole transaction on the first cancel error, so a transient datastore failure here bubbles out after the device has already acknowledgedEraseDevice. That leaves the wipe completed but the pre-wipe queue still intact—the exact state this PR is trying to eliminate. This cleanup should be decoupled from the MDM response path (best-effort with retry/logging, or an async follow-up) instead of returning the error directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/service/apple_mdm.go` around lines 4111 - 4118, The post-wipe cleanup currently runs inside CommandAndReportResults and returns any error from svc.ds.BatchCancelAllHostUpcomingActivities (called after HostByIdentifier and using cmdResult.Identifier()), which can fail and cause the whole MDM response to be treated as failed; change this to best‑effort: after successfully finding host with svc.ds.HostByIdentifier and calling svc.ds.BatchCancelAllHostUpcomingActivities, do not return the error to the caller—log the error with context (including host.ID and cmdResult.Identifier()) and schedule/trigger an async retry or background task for cancellation instead of bubbling the error up; ensure CommandAndReportResults always returns success for the EraseDevice acknowledgement path even if the cancellation step fails.
🧹 Nitpick comments (2)
server/datastore/mysql/software_installers.go (1)
840-842: Avoid intermediate activations while canceling multiple retries.At Line 842, always passing
activateNext=truecan activate queue items mid-loop that are about to be canceled next. Prefer activating only on the last cancellation.♻️ Proposed refactor
- for _, execID := range executionIDs { + for i, execID := range executionIDs { // TODO: pass activateNext: false until the last iteration to avoid // activating activities that are about to be canceled. - if _, err := ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID, true); err != nil { + activateNext := i == len(executionIDs)-1 + if _, err := ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID, activateNext); err != nil { return ctxerr.Wrap(ctx, err, "cancel pending non-policy install retry") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/software_installers.go` around lines 840 - 842, The call to cancelHostUpcomingActivity is always passing activateNext=true which can prematurely activate queue items that will be canceled later; update the loop that calls ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID, true) so it passes activateNext=false for all iterations except the final one (pass true only for the last execID), i.e., detect the last iteration (by index or by checking remaining items) and call ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID, false) for intermediate cancellations and ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID, true) for the final cancellation.server/service/integration_mdm_commands_test.go (1)
205-414: Consider extracting shared helpers for queue setup and post-wipe assertions.These three tests repeat nearly identical enqueue/list/assert and post-wipe verification blocks; a helper would reduce drift and future maintenance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/service/integration_mdm_commands_test.go` around lines 205 - 414, Tests TestWipeMacOSCancelsUpcomingActivities, TestWipeWindowsCancelsUpcomingActivities and TestWipeLinuxCancelsUpcomingActivities duplicate the same enqueue/list/assert and post-wipe verification logic; extract shared helpers to reduce duplication. Add a helper like enqueueTwoScriptsAndAssertUpcoming(t, hostID) that performs the two DoJSON POST /scripts/run calls and the GET /hosts/{id}/activities/upcoming assertions (uses listHostUpcomingActivitiesResponse and fleet.ActivityTypeRanScript), and another helper assertHostWipedAndNoUpcoming(t, hostID) that performs the GET /hosts/{id} MDM DeviceStatus/PendingAction checks and the final GET /hosts/{id}/activities/upcoming empty assertion; for the Linux test keep a small variant helper (enqueueScriptsBehindInFlightWipe) or call enqueueTwoScriptsAndAssertUpcoming after you trigger the wipe so the order is preserved. Replace the repeated blocks in TestWipeMacOSCancelsUpcomingActivities, TestWipeWindowsCancelsUpcomingActivities and TestWipeLinuxCancelsUpcomingActivities with these helpers and keep test-specific MDM simulation code (mdmClient.Idle/Acknowledge, winMDMClient responses, orbit script result) in each test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/datastore/mysql/activities.go`:
- Around line 409-423: The bulk cancel must serialize the host's
upcoming_activities and must not activate the next activity from this path: wrap
the SELECT+loop in a host-level lock (e.g., acquire a row-level advisory lock or
SELECT ... FOR UPDATE on the host's queue) inside withRetryTxx so the snapshot
cannot change while iterating, and change the call to cancelHostUpcomingActivity
so activateNext is always false when invoked from this bulk-cancel path
(remove/ignore the i == len(execIDs)-1 logic); ensure cancelHostUpcomingActivity
still supports activation in its normal single-cancel callers but not when
invoked from this batch routine.
---
Duplicate comments:
In `@server/service/apple_mdm.go`:
- Around line 4111-4118: The post-wipe cleanup currently runs inside
CommandAndReportResults and returns any error from
svc.ds.BatchCancelAllHostUpcomingActivities (called after HostByIdentifier and
using cmdResult.Identifier()), which can fail and cause the whole MDM response
to be treated as failed; change this to best‑effort: after successfully finding
host with svc.ds.HostByIdentifier and calling
svc.ds.BatchCancelAllHostUpcomingActivities, do not return the error to the
caller—log the error with context (including host.ID and cmdResult.Identifier())
and schedule/trigger an async retry or background task for cancellation instead
of bubbling the error up; ensure CommandAndReportResults always returns success
for the EraseDevice acknowledgement path even if the cancellation step fails.
---
Nitpick comments:
In `@server/datastore/mysql/software_installers.go`:
- Around line 840-842: The call to cancelHostUpcomingActivity is always passing
activateNext=true which can prematurely activate queue items that will be
canceled later; update the loop that calls ds.cancelHostUpcomingActivity(ctx,
tx, hostID, execID, true) so it passes activateNext=false for all iterations
except the final one (pass true only for the last execID), i.e., detect the last
iteration (by index or by checking remaining items) and call
ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID, false) for intermediate
cancellations and ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID, true)
for the final cancellation.
In `@server/service/integration_mdm_commands_test.go`:
- Around line 205-414: Tests TestWipeMacOSCancelsUpcomingActivities,
TestWipeWindowsCancelsUpcomingActivities and
TestWipeLinuxCancelsUpcomingActivities duplicate the same enqueue/list/assert
and post-wipe verification logic; extract shared helpers to reduce duplication.
Add a helper like enqueueTwoScriptsAndAssertUpcoming(t, hostID) that performs
the two DoJSON POST /scripts/run calls and the GET
/hosts/{id}/activities/upcoming assertions (uses
listHostUpcomingActivitiesResponse and fleet.ActivityTypeRanScript), and another
helper assertHostWipedAndNoUpcoming(t, hostID) that performs the GET /hosts/{id}
MDM DeviceStatus/PendingAction checks and the final GET
/hosts/{id}/activities/upcoming empty assertion; for the Linux test keep a small
variant helper (enqueueScriptsBehindInFlightWipe) or call
enqueueTwoScriptsAndAssertUpcoming after you trigger the wipe so the order is
preserved. Replace the repeated blocks in
TestWipeMacOSCancelsUpcomingActivities, TestWipeWindowsCancelsUpcomingActivities
and TestWipeLinuxCancelsUpcomingActivities with these helpers and keep
test-specific MDM simulation code (mdmClient.Idle/Acknowledge, winMDMClient
responses, orbit script result) in each test.
🪄 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: 02c3feef-c3a7-48e0-b6b6-7a289f5fb5be
⛔ Files ignored due to path filters (1)
articles/lock-wipe-hosts.mdis excluded by!**/*.md
📒 Files selected for processing (14)
changes/40459-wipe-host-cancels-upcoming-activitiesserver/datastore/mysql/activities.goserver/datastore/mysql/activities_test.goserver/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/scripts.goserver/datastore/mysql/software_installers.goserver/datastore/mysql/vpp_test.goserver/fleet/datastore.goserver/fleet/microsoft_mdm.goserver/mock/datastore_mock.goserver/service/apple_mdm.goserver/service/integration_mdm_commands_test.goserver/service/microsoft_mdm.goserver/service/orbit.go
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the “wipe host cancels all upcoming activities” behavior so that once a Fleet-initiated wipe succeeds, any queued/activated upcoming activities (scripts, software installs/uninstalls, etc.) for that host are silently canceled to prevent them from executing after re-enrollment.
Changes:
- Add datastore support to cancel all upcoming activities for a host in one transaction (
BatchCancelAllHostUpcomingActivities), including already-activated activities. - Trigger upcoming-activity cancellation when wipes succeed across macOS (Apple MDM), Windows (Windows MDM), and Linux (Orbit wipe script result).
- Add integration + datastore tests to validate upcoming activities are cleared after wipe completion.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/orbit.go | Cancels upcoming activities after a successful Orbit-reported wipe script result. |
| server/service/microsoft_mdm.go | Cancels upcoming activities after Windows MDM wipe success is detected. |
| server/service/apple_mdm.go | Cancels upcoming activities after Apple MDM EraseDevice acknowledgement. |
| server/service/integration_mdm_commands_test.go | Adds integration coverage for macOS/Windows/Linux wipe canceling upcoming activities. |
| server/datastore/mysql/activities.go | Adds BatchCancelAllHostUpcomingActivities and activateNext flag to internal cancellation helper. |
| server/datastore/mysql/activities_test.go | Adds unit test verifying batch cancellation for multiple activity types and hosts. |
| server/fleet/datastore.go | Extends fleet.Datastore interface with BatchCancelAllHostUpcomingActivities. |
| server/mock/datastore_mock.go | Updates datastore mock to implement the new interface method. |
| server/fleet/microsoft_mdm.go | Extends Windows MDM save-response result struct with WipeSucceeded. |
| server/datastore/mysql/microsoft_mdm.go | Emits WipeSucceeded result when a wipe response is successfully processed. |
| server/datastore/mysql/scripts.go | Updates internal cancellation call sites for new activateNext signature. |
| server/datastore/mysql/software_installers.go | Updates internal cancellation call site for new activateNext signature. |
| server/datastore/mysql/vpp_test.go | Updates direct calls to cancelHostUpcomingActivity with new activateNext argument. |
| changes/40459-wipe-host-cancels-upcoming-activities | Release note for user-visible wipe behavior change. |
| articles/lock-wipe-hosts.md | Documents that wipe silently cancels upcoming activities without adding canceled-history entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/datastore/mysql/activities.go (1)
417-423:⚠️ Potential issue | 🟠 MajorAvoid re-activating work from wipe batch-cancel path.
Line 422/Line 423 can still activate a “late” queued activity in a wipe flow. For wipe semantics, batch cancel should not activate next activity at all.
Suggested change
- activateNext := i == len(execIDs)-1 + activateNext := false details, err := ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID, activateNext)#!/bin/bash # Verify whether any enqueue paths can still add upcoming activities after wipe states, # and confirm current call sites of BatchCancelAllHostUpcomingActivities. set -euo pipefail echo "== Call sites of BatchCancelAllHostUpcomingActivities ==" rg -nP --type=go '\bBatchCancelAllHostUpcomingActivities\s*\(' -C2 echo echo "== Insert paths into upcoming_activities ==" rg -nP --type=go 'INSERT\s+INTO\s+upcoming_activities' -C6 echo echo "== Wipe-state guards near enqueue-related code ==" rg -nP --type=go '\b(PendingActionWipe|DeviceStatusWiped|WellKnownActionWipe)\b' -C3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/activities.go` around lines 417 - 423, The loop in BatchCancelAllHostUpcomingActivities currently computes activateNext := i == len(execIDs)-1 and passes it into ds.cancelHostUpcomingActivity, which can re-activate a late queued activity during a wipe; change the logic to never activate the next activity in this wipe batch-cancel path by always passing false (e.g., set activateNext := false or inline false) when calling ds.cancelHostUpcomingActivity in the execIDs loop so BatchCancelAllHostUpcomingActivities never re-activates work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/datastore/mysql/activities.go`:
- Around line 417-423: The loop in BatchCancelAllHostUpcomingActivities
currently computes activateNext := i == len(execIDs)-1 and passes it into
ds.cancelHostUpcomingActivity, which can re-activate a late queued activity
during a wipe; change the logic to never activate the next activity in this wipe
batch-cancel path by always passing false (e.g., set activateNext := false or
inline false) when calling ds.cancelHostUpcomingActivity in the execIDs loop so
BatchCancelAllHostUpcomingActivities never re-activates work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6b7f49b-33b9-46db-ae13-4aa90074f993
📒 Files selected for processing (1)
server/datastore/mysql/activities.go
Member
|
@JordanMontgomery Let me know if you want me to pick up any changes here |
Member
@MagnusHJensen yeah if you don't mind. I assigned it to me just to make suer it keeps moving. If you wanna grab it and help move it the rest of the way that'd be great and I can review |
Member
|
@JordanMontgomery I'll do that, maybe if you have time to just comment back on my one question regarding Apple wipe, and also triggering this behaviour on error, since we know it will always wipe, obliterate or not. I'll pick this up, and do some final changes, but it's looking pretty good. |
MagnusHJensen
temporarily deployed
to
Docker Hub
April 30, 2026 21:21 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/service/apple_mdm.go`:
- Around line 4112-4119: The EraseDevice branch currently cancels upcoming
activities whenever the command result is succeeded or errored, which
incorrectly cancels work for user-enrolled devices where EraseDevice always
fails; change the logic around svc.ds.BatchCancelAllHostUpcomingActivities so
you only cancel when a wipe truly occurred or was attempted: fetch the host via
svc.ds.HostByIdentifier(r.Context, cmdResult.Identifier()) and then either (A)
skip cancellation if the host is user-enrolled (inspect Host.EnrollmentType or a
boolean like Host.IsUserEnrolled) or (B) only cancel when
UpdateHostLockWipeStatusFromAppleMDMResult (or error chain inspection using
errors.Is/As) signals a concrete wipe attempt/terminal wipe state (introduce or
use a sentinel error or specific status value) before calling
svc.ds.BatchCancelAllHostUpcomingActivities; adjust EraseDevice handling
accordingly so false-positive cancellations are avoided.
🪄 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: d0637269-6874-4181-a736-7c37dd835d84
📒 Files selected for processing (2)
server/datastore/mysql/software_installers.goserver/service/apple_mdm.go
MagnusHJensen
temporarily deployed
to
Docker Hub
April 30, 2026 21:38 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Member
|
@JordanMontgomery If you have time to review this, it could be great, then I'll maybe start the clear on re-enrollment story work Monday. Just tested on mac, that it cleared, and it works as expected. |
JordanMontgomery
approved these changes
May 1, 2026
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.
Related issue: Resolves #40459
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
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
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
Recording: https://drive.google.com/file/d/1_XqLyy-oY-WnIa97R4t9HihiBq3Fui6n/view?usp=drive_link
Summary by CodeRabbit
New Features
Bug Fixes
Tests