From a2701450c8669949694bbdecf1b7cdbac0be6b46 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Tue, 12 May 2026 03:52:28 +0000 Subject: [PATCH] fix(infra): status-reaper + watchdog read per-entry `status` not `state` (Gitea 1.22.6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gitea 1.22.6 combined-status API returns per-entry objects with a `status` key (NOT `state`). The top-level `state` field is the aggregate; per-entry has only `status`. Revisions 1–4 of status-reaper read `s.get("state")` per-entry and got None for every entry — their compensation logic was unreachable. The watchdog's `is_red()` and `render_body()` had the same bug at 3 sites, silently missing all per-context failures. Fix: `s.get("status") or s.get("state") or ""` at every per-entry read site. Top-level combined-state reads are unaffected. Tests: 8 new cases (4 reaper + 4 watchdog) covering canonical `status`-key shape, `status`-over-`state` precedence, backward compat with `state`-only shape, and non-failure passthrough. 91/91 tests pass. Refs: molecule-core#652 --- .gitea/scripts/main-red-watchdog.py | 6 +- .gitea/scripts/status-reaper.py | 2 +- tests/test_main_red_watchdog.py | 80 ++++++++++++++++++ tests/test_status_reaper.py | 127 ++++++++++++++++++++++++++++ 4 files changed, 211 insertions(+), 4 deletions(-) diff --git a/.gitea/scripts/main-red-watchdog.py b/.gitea/scripts/main-red-watchdog.py index 85e4de36..ac2b4712 100755 --- a/.gitea/scripts/main-red-watchdog.py +++ b/.gitea/scripts/main-red-watchdog.py @@ -224,7 +224,7 @@ def is_red(status: dict) -> tuple[bool, list[dict]]: red_states = {"failure", "error"} failed = [ s for s in statuses - if isinstance(s, dict) and s.get("state") in red_states + if isinstance(s, dict) and (s.get("status") or s.get("state") or "") in red_states ] return (combined in red_states or bool(failed), failed) @@ -313,7 +313,7 @@ def render_body(sha: str, failed: list[dict], debug: dict) -> str: else: for s in failed: ctx = s.get("context", "(no context)") - state = s.get("state", "(no state)") + state = s.get("status") or s.get("state") or "(no state)" url = s.get("target_url") or "" desc = (s.get("description") or "").strip() entry = f"- **{ctx}** — `{state}`" @@ -546,7 +546,7 @@ def run_once(*, dry_run: bool = False) -> int: "combined_state": status.get("state"), "failed_contexts": [s.get("context") for s in failed], "all_contexts": [ - {"context": s.get("context"), "state": s.get("state")} + {"context": s.get("context"), "state": s.get("status") or s.get("state")} for s in (status.get("statuses") or []) if isinstance(s, dict) ], diff --git a/.gitea/scripts/status-reaper.py b/.gitea/scripts/status-reaper.py index 41cc8c1f..9b09c69b 100644 --- a/.gitea/scripts/status-reaper.py +++ b/.gitea/scripts/status-reaper.py @@ -447,7 +447,7 @@ def reap( if not isinstance(s, dict): continue context = s.get("context") or "" - state = s.get("state") or "" + state = s.get("status") or s.get("state") or "" # Only `failure` is the bug shape. `error`/`pending`/`success` # left alone — they have other meanings. diff --git a/tests/test_main_red_watchdog.py b/tests/test_main_red_watchdog.py index 1b14fe27..0070b22f 100644 --- a/tests/test_main_red_watchdog.py +++ b/tests/test_main_red_watchdog.py @@ -624,3 +624,83 @@ def test_require_runtime_env_exits_when_missing(wd_module, monkeypatch): with pytest.raises(SystemExit) as excinfo: wd_module._require_runtime_env() assert excinfo.value.code == 2 + + +# -------------------------------------------------------------------------- +# Gitea 1.22.6 per-entry `status` key (not `state`) — issue #652 +# -------------------------------------------------------------------------- +# Gitea 1.22.6 combined-status API returns per-entry objects with a +# `status` key (NOT `state`). is_red's per-entry check read +# `s.get("state")` which returned None for every entry, making the +# per-context red-detection dead code. Fix: `status or state` fallback. +# +# Canonical Gitea 1.22.6 per-entry shape: +# {"context": "...", "status": "failure", "target_url": "..."} +# (no `"state"` key per-entry; `"state"` exists only at top-level.) + + +def test_is_red_detects_canonical_status_key_per_entry(wd_module): + """is_red must detect failure via the canonical `status` key + (not `state`) for per-entry statuses.""" + red, failed = wd_module.is_red(_combined_status("failure", [ + {"context": "ci/test", "status": "failure"}, + ])) + assert red is True + assert len(failed) == 1 + assert failed[0]["context"] == "ci/test" + + +def test_is_red_status_key_takes_precedence_over_state_per_entry(wd_module): + """If both `status` and `state` are present per-entry, `status` + (canonical) wins. A watchdog reading `state` would miss this.""" + red, failed = wd_module.is_red(_combined_status("failure", [ + { + "context": "ci/test", + "status": "failure", # canonical — should trigger + "state": "success", # old key — should be IGNORED + }, + ])) + assert red is True + assert failed[0]["context"] == "ci/test" + assert failed[0].get("status") == "failure" + + +def test_is_red_backward_compat_state_key_per_entry(wd_module): + """Old-style per-entry objects using `state` key still work. + Fallback `state` must not be broken by the `status`-key fix.""" + red, failed = wd_module.is_red(_combined_status("failure", [ + {"context": "ci/test", "state": "failure"}, # old key — must still work + ])) + assert red is True + assert len(failed) == 1 + + +def test_render_body_uses_status_key_for_entry_state(wd_module): + """render_body reads per-entry `s.get("status") or s.get("state")` + so the issue body correctly shows the state for Gitea-1.22.6-style + entries that only have `status` (no `state` key).""" + entries = [ + { + "context": "ci/test", + "status": "failure", + "target_url": "https://ci.example/run/42", + "description": "tests failed", + }, + { + "context": "ci/lint", + "status": "error", + "description": "linter crashed", + }, + ] + body = wd_module.render_body( + SHA_RED, + failed=entries, + debug={}, + ) + assert "ci/test" in body + assert "ci/lint" in body + assert "failure" in body + assert "error" in body + assert "tests failed" in body + assert "linter crashed" in body + diff --git a/tests/test_status_reaper.py b/tests/test_status_reaper.py index 72dc9690..079fdae0 100644 --- a/tests/test_status_reaper.py +++ b/tests/test_status_reaper.py @@ -773,3 +773,130 @@ def test_reap_continues_on_per_sha_apierror(sr_module, monkeypatch, capsys): captured = capsys.readouterr() assert "::warning::" in captured.out or "::notice::" in captured.out assert SHA_A[:10] in captured.out + + +# -------------------------------------------------------------------------- +# rev4: Gitea 1.22.6 per-entry `status` key (not `state`) +# -------------------------------------------------------------------------- +# Gitea 1.22.6 combined-status API returns per-entry objects with a +# `status` key (NOT `state`). All prior revisions (rev1–rev3) read +# `s.get("state")` per-entry and got None for every entry — their +# compensation code was unreachable. This test group exercises the fix: +# `state = s.get("status") or s.get("state") or ""`. +# +# Canonical Gitea 1.22.6 per-entry shape: +# {"context": "...", "status": "failure", "target_url": "..."} +# (no `"state"` key per-entry; `"state"` exists only at the top level.) + + +def test_reap_compensates_canonical_status_key(sr_module, monkeypatch): + """Gitea 1.22.6: per-entry object has `status` key (not `state`). + reap() must detect failure via the canonical `status` key and + compensate schedule-only workflows.""" + calls = [] + + def fake_api(method, path, *, body=None, query=None, expect_json=True): + calls.append((method, path, body)) + return (201, {}) + + monkeypatch.setattr(sr_module, "api", fake_api) + + workflow_map = {"drift": False} # schedule-only → compensable + # Gitea 1.22.6 canonical shape: `status` key, NO `state` key per-entry. + combined = { + "state": "failure", # top-level aggregate + "statuses": [ + { + "context": "drift / drift (push)", + "status": "failure", # canonical key (no `state` key) + "target_url": "https://example.test/run/42", + } + ], + } + counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False) + assert counters["compensated"] == 1 + assert counters["compensated_contexts"] == ["drift / drift (push)"] + assert len(calls) == 1 + assert calls[0][0] == "POST" + + +def test_reap_status_key_takes_precedence_over_state_key(sr_module, monkeypatch): + """If both `status` and `state` are present per-entry, `status` + (canonical) wins. This catches a reaper that reads the wrong key.""" + calls = [] + + def fake_api(method, path, *, body=None, query=None, expect_json=True): + calls.append((method, path, body)) + return (201, {}) + + monkeypatch.setattr(sr_module, "api", fake_api) + + workflow_map = {"sweep-cf-tunnels": False} + # Both keys present; `status=failure` is the canonical value. + combined = { + "state": "failure", + "statuses": [ + { + "context": "sweep-cf-tunnels / sweep (push)", + "status": "failure", # canonical — should trigger comp. + "state": "success", # old-key — should be IGNORED + "target_url": "https://x/y", + } + ], + } + counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False) + # MUST compensate (canonical status=failure takes precedence). + assert counters["compensated"] == 1 + assert len(calls) == 1 + + +def test_reap_backward_compat_state_key_still_works(sr_module, monkeypatch): + """Old-style per-entry objects using `state` key (pre-1.22.6 or + mixed-API responses) still compensate. Fallback `s.get("state")` + must not be broken by the `status`-key fix.""" + calls = [] + + def fake_api(method, path, *, body=None, query=None, expect_json=True): + calls.append((method, path, body)) + return (201, {}) + + monkeypatch.setattr(sr_module, "api", fake_api) + + workflow_map = {"drift": False} + # Old shape: only `state` key per-entry (no `status`). + combined = { + "state": "failure", + "statuses": [ + { + "context": "drift / drift (push)", + "state": "failure", # old key — must still work + "target_url": "https://x/y", + } + ], + } + counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False) + assert counters["compensated"] == 1 + assert len(calls) == 1 + + +def test_reap_ignores_canonical_status_non_failure(sr_module, monkeypatch): + """`status: pending/success/error` per-entry must NOT be compensated. + Only `status: failure` (the Gitea 1.22.6 bug shape) triggers comp.""" + monkeypatch.setattr( + sr_module, "api", + lambda *a, **kw: (_ for _ in ()).throw( + AssertionError("api should not be called for non-failure") + ), + ) + workflow_map = {"x": False} + combined = { + "state": "failure", + "statuses": [ + {"context": "x / job (push)", "status": "pending"}, + {"context": "x / job (push)", "status": "success"}, + {"context": "x / job (push)", "status": "error"}, + ], + } + counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False) + assert counters["compensated"] == 0 + assert counters["preserved_non_failure"] == 3 -- 2.45.2