Clear stale failure detail when an Android profile becomes verified

[Dhvanit41]

Related issue: Resolves #50357

When an Android configuration profile transitioned from failed back to verified,
verifyDevicePolicy copied the stored detail into the payload it upserted, so the
row kept the old failure message even though the profile was now compliant. The stale
message stayed visible against a verified profile.

This clears detail whenever a profile is marked verified, in both paths of
verifyDevicePolicy:

• the path taken when the device reports no non-compliance at all, and
• the per-profile path, where only some profiles are still non-compliant (a profile
  that is no longer in NonComplianceDetails is verified and must not keep the
  message from the previous report).

Profiles that are still non-compliant are unaffected: they continue to be marked
failed with a freshly built message.

The datastore already writes detail = VALUES(detail) on this path, so passing an
empty string clears the column. The preserveExistingDetail variant of the upsert —
which intentionally keeps a forward-looking detail such as "Waiting for certificate…"
on withheld ONC profiles — uses a different caller and is untouched.

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/ or ee/fleetd-chrome/changes.
  See Changes files for more information.

Testing

• Added/updated automated tests
• QA'd all new/changed functionality manually

Added TestStatusReportPolicyValidation/stale failure detail is cleared once a profile is verified, covering both branches: a profile that becomes compliant while another
stays non-compliant, and a report with no non-compliance at all.

Verified the test is a genuine regression test — with the fix reverted it fails with
the exact detail string from the issue:

Error: Should be empty, but was "passwordPolicies" setting couldn't apply to a host.
Reason: USER_ACTION. Other settings are applied.

go test ./server/mdm/android/... passes, gofmt and go vet are clean.

Not manually QA'd: reproducing it end to end needs a BYOD Android device that can
trip a USER_ACTION non-compliance and then satisfy it, which I don't have access
to. The unit test reproduces the reported database state instead.

Summary by CodeRabbit

• Bug Fixes

  • Cleared outdated error details when Android MDM profiles become compliant.
  • Refreshed failure details for profiles that remain non-compliant.
  • Removed stale failure details when no policy violations remain.

• Tests

  • Added regression coverage for compliance status and error-detail updates.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a9ddbcc2-f879-418a-bdde-6349a713b88c

📥 Commits

Reviewing files that changed from the base of the PR and between 53d7b56 and bf2f392.

📒 Files selected for processing (1)

• server/mdm/android/service/pubsub_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• server/mdm/android/service/pubsub_test.go

---

Walkthrough

Android MDM profile verification now clears stored failure details when a profile becomes verified. Non-compliant profiles receive refreshed failure details. Regression coverage tests selective status transitions and clearing all details when the device reports full compliance.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: clearing stale Android profile failure details after verification.
Description check	✅ Passed	The description identifies the issue, explains the fix, documents testing, and clearly notes that manual QA was not performed.
Linked Issues check	✅ Passed	The changes satisfy issue #50357 by clearing stale detail values for verified Android profiles while preserving refreshed failure details.
Out of Scope Changes check	✅ Passed	The code and test changes are limited to the Android profile verification behavior required by issue #50357.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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/mdm/android/service/pubsub_test.go`:
- Around line 920-922: Update the stillFailingProfile assertion in the relevant
pubsub test to verify that profile.Detail contains the refreshed cameraDisabled
failure message, or at minimum differs from staleDetail, rather than only being
non-empty. Preserve the existing MDMDeliveryFailed status assertion.

🪄 Autofix

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 Plus

Run ID: f1d8c1ca-1bc0-4c17-82ab-14f5e062ea09

📥 Commits

Reviewing files that changed from the base of the PR and between 12754cc and 53d7b56.

⛔ Files ignored due to path filters (1)

• changes/50357-android-profile-stale-failure-detail.md is excluded by !**/*.md

📒 Files selected for processing (2)

• server/mdm/android/service/pubsub.go
• server/mdm/android/service/pubsub_test.go

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.40%. Comparing base (12754cc) to head (bf2f392).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #50774      +/-   ##
==========================================
- Coverage   68.40%   68.40%   -0.01%     
==========================================
  Files        3963     3963              
  Lines      255174   255174              
  Branches    13616    13616              
==========================================
- Hits       174555   174554       -1     
- Misses      65020    65023       +3     
+ Partials    15599    15597       -2     

Flag	Coverage Δ
backend	69.61% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Dhvanit41]

@georgekarrv this one's ready for a look when you get a chance.

Fix for #50357 — verifyDevicePolicy was carrying the stored Detail forward when marking a profile verified, so a previously failed profile kept its old error message. Cleared it in both paths (no non-compliance reported, and the per-profile path), with a regression test in TestStatusReportPolicyValidation.

Also addressed the CodeRabbit comment in bf2f393 — the still-failing assertion was only checking NotEmpty, so it now asserts the exact rebuilt message instead.

Let me know if you'd like anything changed.