fix: mark alerts dropped by the status filter as skipped in alerts_v2

[entity-0-0-2]

Alerts for skipped tests are inserted into alerts_v2 and then stay pending forever: the default status filter drops them before sending and nothing marks them skipped afterwards, so they are never sent and never cleaned up (#2385).

run_alerts filters the fetched alerts in memory (data_monitoring_alerts.py:433) and only passes survivors to _skip_alerts (line 437), so the terminal UPDATE never fires for filtered-out rows. The default filter (schema.py:138-148, from 47f8b02 in 2023) includes FAIL, ERROR and WARN but not SKIPPED, while population does insert skipped alerts.

run_alerts now collects the ids the filter dropped and passes them to _skip_alerts, so every fetched alert reaches a terminal state. A unit test asserts filtered-out alerts are marked skipped: fails on main, passes with the change. The monitor unit suite passes (95 tests).

Fixes #2385

[github-actions]

👋 @entity-0-0-2
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 85bcd52f-1f68-47c9-abd6-e0c207ad2f81

---

📥 Commits

Reviewing files that changed from the base of the PR and between fd9872a and fdf699a.

---

📒 Files selected for processing (2)

• elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py
• tests/unit/monitor/data_monitoring/alerts/test_data_monitoring_alerts.py

---

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

---

📝 Walkthrough

Walkthrough

The alert monitor now marks alerts excluded by filtering as skipped. A new test checks that fetched alerts reach either the send or skip path.

Changes

Alert filtering and status updates

Layer / File(s)	Summary
Filter alerts and update skipped status elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py, tests/unit/monitor/data_monitoring/alerts/test_data_monitoring_alerts.py	run_alerts sorts only alerts retained by filtering. It marks filter-excluded alerts as skipped along with alerts marked for skipping by sorting. The test checks that all fetched alerts reach a send or skip path.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: guyeshdat

---

Merge Risk | ⚪ Minimal · up to fdf69

Merge Risk: ⚪ Minimal · up to fdf69

Alerts removed by the status filter will now be marked skipped instead of staying pending. Sending behavior is unchanged, and no merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to fdf69

A filtered monitoring run can permanently suppress valid alerts intended for a later run using different filters. The impact is bounded to runs sharing the same alert history and requires existing monitoring access, but it changes alert ownership and recovery behavior.

Retained concerns

• Medium · reliability · inferred: Run-specific selection now terminalizes alerts outside that run's selected scope. For example, a run filtering owners:alice can mark otherwise eligible alerts for Bob skipped; a later Bob-filtered run cannot fetch them because retrieval requires pending status. At the base, filter-rejected rows remained pending. Concurrent differently filtered runs can likewise let a skip win before another run records a successful send. This changes shared-state ownership and failure containment, rather than merely cleaning up intrinsically skipped results. The pending-status guard protects already terminal rows but does not resolve cross-run ownership.

Security review details

Security Blast Radius

• inferred — The affected scope can include otherwise eligible pending alerts across the targeted alerts_v2 table within the fetch and update time limits, including alerts outside the chosen owner, model, or tag. Interference requires consumers sharing that table; exposure across separately configured data stores or tenants is not established.

Security Findings and Attack Paths

• inferred — A caller with existing monitoring execution and warehouse access can use a narrow filter to suppress alerts outside that selection from subsequent runs. This is a newly expanded disposition effect, not demonstrated unauthenticated access or a gain of database privileges.

Trust Boundaries and Controls

• observed — Skipped and sent writers require matching alert IDs, pending status, and an eligible timestamp. These existing controls make repeated terminal updates harmless to already terminal rows, but neither writer binds disposition to an invocation, selector, or delivery destination.

Resilience and Maintainability Implications

• inferred — If two differently filtered runs fetch the same pending alert, one can now mark it skipped while the other sends it. The later sent update cannot transition a skipped row, allowing durable history to disagree with delivery. External sending and status updates were already separate at the base; the PR newly exposes filter-rejected alerts to this competing transition.

Hardening Proposals

• proposed — Separate permanent result-status ineligibility from per-run selection exclusions. If independently filtered consumers are supported, define per-consumer disposition or an ownership claim before globally terminalizing alerts.

---

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Issue #2385 requires alerts removed by the status filter to reach a terminal state. The change preserves the filtered list for sorting, computes filtered-out alerts, and passes those alerts to `skip…
Out of Scope Changes check		The pull request changes only run_alerts and its unit tests. The implementation directly supports issue #2385. The test verifies the required terminal-state behavior. No unrelated production behavio…
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: marking alerts removed by the status filter as skipped in alerts_v2.

---

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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