From 70eb2531903c2163ee57a3eebfdc7ece45c5c55a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 15 Oct 2026 13:58:24 -0400 Subject: [PATCH] fix(mcp): the overview drops an Ansible failure a later run put right (#5842) get_overview listed every failed or interrupted run of the last 24 hours, so a dry run that failed, was fixed and re-run green kept warning for a day. A failure now drops out once a later success of the same playbook, source and hosts is at least as real: an applied success clears either kind of failure, a dry-run success only a dry-run failure. Co-Authored-By: Claude Opus 5.5 --- steward/mcp/tools/overview.py | 25 +++++++++++++++++++++++++ tests/mcp/test_tools.py | 19 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/steward/mcp/tools/overview.py b/steward/mcp/tools/overview.py index 67b5439..4e0e742 100644 --- a/steward/mcp/tools/overview.py +++ b/steward/mcp/tools/overview.py @@ -179,6 +179,24 @@ def agent_findings(hosts, agent: dict, thresholds: dict, return out +def _run_key(r) -> tuple: + return (r.source_name, r.playbook_path, r.inventory_scope) + + +def _dry(r) -> bool: + return bool((r.params or {}).get("check")) + + +def still_failing(failed, succeeded) -> list: + """The failed runs nothing has since put right: a later success of the + same playbook from the same source on the same hosts clears one, as long + as it was at least as real (a dry run's success clears only a dry run's + failure).""" + return [f for f in failed if not any( + _run_key(s) == _run_key(f) and s.started_at > f.started_at and (_dry(f) or not _dry(s)) + for s in succeeded)] + + def run_findings(runs) -> list[dict]: out = [] for r in runs: @@ -292,6 +310,13 @@ async def get_overview() -> dict: .order_by(AnsibleRun.started_at.desc()) .limit(20) )).scalars()) + if runs: + runs = still_failing(runs, (await db.execute( + select(AnsibleRun) + .where(AnsibleRun.status == AnsibleRunStatus.success, + AnsibleRun.started_at > min(r.started_at for r in runs), + AnsibleRun.playbook_path.in_({r.playbook_path for r in runs})) + )).scalars()) windows = await alert_svc.list_maintenance_windows(db) open_anomalies = await anom.open_anomalies(db) to_review = await anom.to_review_count(db) diff --git a/tests/mcp/test_tools.py b/tests/mcp/test_tools.py index 77a4777..cf246c2 100644 --- a/tests/mcp/test_tools.py +++ b/tests/mcp/test_tools.py @@ -195,6 +195,25 @@ def test_run_findings_quote_the_failures(): assert f["kind"] == "ansible_run_failed" and "apt lock held" in f["summary"] +def _run(status, ago, *, check=False, scope="steward:target:util", playbook="upkeep/x.yml"): + return NS(id=f"{status}-{ago}", source_name="steward-local", playbook_path=playbook, + inventory_scope=scope, status=status, started_at=NOW - timedelta(minutes=ago), + params={"check": check}) + + +def test_a_failed_run_put_right_since_is_not_a_finding(): + F, S = AnsibleRunStatus.failed, AnsibleRunStatus.success + dry_fail, real_fail = _run(F, 60, check=True), _run(F, 50) + # A dry run fixed and re-run dry: the dry failure is cleared, the real one is not. + assert overview.still_failing([dry_fail, real_fail], [_run(S, 55, check=True)]) == [real_fail] + # A dry success after a real failure doesn't clear it; a real success does. + assert overview.still_failing([real_fail], [_run(S, 40, check=True)]) == [real_fail] + assert overview.still_failing([real_fail], [_run(S, 40)]) == [] + # A success elsewhere, or before the failure, clears nothing. + assert overview.still_failing([real_fail], [_run(S, 40, scope="steward:target:other"), + _run(S, 40, playbook="upkeep/y.yml"), _run(S, 70)]) == [real_fail] + + def test_rank_puts_critical_first_then_oldest(): fs = [overview.finding("warning", "k", "w-old", "", since=NOW - timedelta(days=1)), overview.finding("critical", "k", "c-new", "", since=NOW - timedelta(minutes=1)),