From 6fa62c9520184a8d73b05085029a8878aa23a14d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 15 Oct 2026 14:52:19 -0400 Subject: [PATCH] fix(mcp): read the successes once, so every failed run is checked against all of them (#5842) still_failing was handed the query's one-pass result. The first failed run's any() consumed it, so a second failure in the window found no successes left and was reported though a later run had put it right (ci-runner-volumes came back once keepalived-survey also had a failure). Co-Authored-By: Claude Opus 5.5 --- steward/mcp/tools/overview.py | 3 ++- tests/mcp/test_tools.py | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/steward/mcp/tools/overview.py b/steward/mcp/tools/overview.py index 4e0e742..9b0ee83 100644 --- a/steward/mcp/tools/overview.py +++ b/steward/mcp/tools/overview.py @@ -191,7 +191,8 @@ 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).""" + failure). `succeeded` may be a one-pass result, so it is read once.""" + succeeded = list(succeeded) 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)] diff --git a/tests/mcp/test_tools.py b/tests/mcp/test_tools.py index cf246c2..92f89df 100644 --- a/tests/mcp/test_tools.py +++ b/tests/mcp/test_tools.py @@ -214,6 +214,15 @@ def test_a_failed_run_put_right_since_is_not_a_finding(): _run(S, 40, playbook="upkeep/y.yml"), _run(S, 70)]) == [real_fail] +def test_every_failure_is_checked_against_every_success_even_from_a_one_pass_result(): + """The live miss: a query result is an iterator, and the first failure's + check used it up, so a second failure was never cleared.""" + F, S = AnsibleRunStatus.failed, AnsibleRunStatus.success + a, b = _run(F, 60, check=True, playbook="upkeep/a.yml"), _run(F, 50, check=True, playbook="upkeep/b.yml") + successes = iter([_run(S, 45, check=True, playbook="upkeep/b.yml"), _run(S, 55, check=True, playbook="upkeep/a.yml")]) + assert overview.still_failing([b, a], successes) == [] + + 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)),