diff --git a/.gitea/scripts/main-red-watchdog.py b/.gitea/scripts/main-red-watchdog.py index 85e4de36..a8467456 100755 --- a/.gitea/scripts/main-red-watchdog.py +++ b/.gitea/scripts/main-red-watchdog.py @@ -222,9 +222,20 @@ def is_red(status: dict) -> tuple[bool, list[dict]]: combined = status.get("state") statuses = status.get("statuses") or [] red_states = {"failure", "error"} + # Schema asymmetry: top-level combined uses `state`, but per-entry + # items in `statuses[]` use `status` in Gitea 1.22.6. Prefer + # `status`; fall back to `state` defensively. Verified empirically + # 2026-05-12 03:42Z. Pre-rev4 code only read `state` from per-entry + # items → failed[] always empty → render_body always showed the + # "no per-context entries were in a red state" fallback even when + # the combined-state correctly flagged red. See + # `feedback_smoke_test_vendor_truth_not_shape_match`. + def _entry_state(s: dict) -> str: + return s.get("status") or s.get("state") or "" + failed = [ s for s in statuses - if isinstance(s, dict) and s.get("state") in red_states + if isinstance(s, dict) and _entry_state(s) in red_states ] return (combined in red_states or bool(failed), failed) @@ -313,7 +324,9 @@ 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)") + # Per-entry key is `status` in Gitea 1.22.6, not `state` + # (see _entry_state in is_red). Fallback for forward-compat. + 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 +559,11 @@ 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")} + # Per-entry key is `status` in Gitea 1.22.6, not `state`. + # Pre-rev4 debug output reported `state: None` for every + # context, making run logs useless for triage. + {"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 5e1f895f..9833e7b4 100644 --- a/.gitea/scripts/status-reaper.py +++ b/.gitea/scripts/status-reaper.py @@ -452,7 +452,18 @@ def reap( if not isinstance(s, dict): continue context = s.get("context") or "" - state = s.get("state") or "" + # Schema asymmetry: Gitea 1.22.6 returns the TOP-LEVEL combined + # aggregate as `combined.state` but each per-context entry in + # `combined.statuses[]` uses the key `status`, NOT `state`. + # Prefer `status`; fall back to `state` so a future Gitea + # version (or a test fixture written against the wrong key) + # still flows through the compensation path. Verified empirically + # via direct API probe 2026-05-12 03:42Z: + # /repos/.../commits/{sha}/status entries → key is "status". + # Pre-rev4 code read "state" only → returned "" → bypassed the + # `state != "failure"` guard → compensation path unreachable. + # See `feedback_smoke_test_vendor_truth_not_shape_match`. + 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..ab3f4bb9 100644 --- a/tests/test_main_red_watchdog.py +++ b/tests/test_main_red_watchdog.py @@ -189,6 +189,78 @@ def test_is_red_no_statuses(wd_module): assert failed == [] +# -------------------------------------------------------------------------- +# Per-entry vendor-truth key (rev4) — see status-reaper rev4 sibling +# +# Gitea 1.22.6 returns per-entry items in combined.statuses[] with key +# `status`, not `state`. Pre-rev4 code only read `state` → failed[] +# was always empty → render_body always emitted the fallback "no +# per-context entries were in a red state". These tests use the +# canonical Gitea shape to lock the fix in. +# -------------------------------------------------------------------------- +def test_is_red_vendor_truth_status_key_under_pending(wd_module): + """Real Gitea 1.22.6 shape: per-entry uses `status`. A single failed + context counts as red even when combined is `pending`. Pre-rev4 + this returned `(False, [])` because `s.get("state")` was None.""" + red, failed = wd_module.is_red({ + "state": "pending", + "statuses": [ + {"context": "ci/lint", "status": "success"}, + {"context": "ci/test", "status": "failure"}, + {"context": "ci/build", "status": "pending"}, + ], + }) + assert red is True + assert [s["context"] for s in failed] == ["ci/test"] + + +def test_is_red_status_takes_precedence_over_state(wd_module): + """If both keys present (defensive), `status` (vendor truth) wins.""" + red, failed = wd_module.is_red({ + "state": "pending", + "statuses": [ + # `status=failure` is truth even though `state=success` is + # stale. Locking in the precedence prevents a hypothetical + # future Gitea release that emits both from re-introducing + # the bug under a different shape. + {"context": "ci/test", "status": "failure", "state": "success"}, + ], + }) + assert red is True + assert len(failed) == 1 + + +def test_is_red_state_only_fallback_still_works(wd_module): + """Backward-compat: a legacy fixture or future Gitea variant that + only emits `state` still trips the red detection via the fallback + chain. Keeps pre-rev4 fixtures green during the rev4 rollout.""" + red, failed = wd_module.is_red({ + "state": "pending", + "statuses": [ + {"context": "ci/test", "state": "failure"}, # legacy shape + ], + }) + assert red is True + assert len(failed) == 1 + + +def test_render_body_uses_status_key_for_per_entry_state(wd_module): + """render_body must surface the per-entry `status` value in the + issue body. Pre-rev4 it read `state` (always None on real Gitea) → + every issue body said `(no state)`, defeating the diagnostic.""" + failed = [ + {"context": "ci/test", "status": "failure", + "target_url": "https://example.test/run/1", + "description": "broke"}, + ] + body = wd_module.render_body("deadbeefcafe1234", failed, {}) + assert "`failure`" in body, ( + "render_body did not surface per-entry status — likely still " + "reading `state` key only (rev1-3 bug)." + ) + assert "(no state)" not in body + + # -------------------------------------------------------------------------- # Happy path — main is green, no issue created # -------------------------------------------------------------------------- diff --git a/tests/test_status_reaper.py b/tests/test_status_reaper.py index fda532c7..81327487 100644 --- a/tests/test_status_reaper.py +++ b/tests/test_status_reaper.py @@ -544,6 +544,156 @@ def test_reap_unparseable_push_context_preserved(sr_module, monkeypatch): assert counters["preserved_unparseable"] == 1 +# -------------------------------------------------------------------------- +# Per-context status-key vendor-truth (rev4) +# +# Gitea 1.22.6 returns commit-status entries with key `status` per entry, +# NOT `state`. The TOP-LEVEL combined aggregate uses `state`. This schema +# asymmetry caused rev1-3 to take the compensation path 0 times despite +# triggering on real failures: `s.get("state")` returned None → state +# evaluated to "" → `"" != "failure"` guard preserved every entry. +# +# These tests explicitly use the vendor-truth shape (`status` per entry), +# proving the rev4 fix routes the failure entry through compensation. +# Fixtures in rev1-3 tests above use `state` (the pre-fix bug shape) — +# we keep them for backward-compat coverage via the fallback in +# `s.get("status") or s.get("state")`, but the canonical Gitea shape +# uses `status`. Logged under +# `feedback_smoke_test_vendor_truth_not_shape_match`. +# -------------------------------------------------------------------------- +def test_reap_per_context_uses_status_key_not_state(sr_module, monkeypatch): + """Empirical Gitea 1.22.6 shape: per-entry uses `status`, top-level + uses `state`. The rev4 fix MUST detect failure via `status`.""" + 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 = {"staging-smoke": False} # no push trigger → Class-O + # Real Gitea-shaped response: top-level `state`, per-entry `status`. + # No `state` key on the per-entry item. + combined = { + "state": "failure", + "statuses": [ + { + "context": "staging-smoke / smoke (push)", + "status": "failure", # ← vendor-truth key + "target_url": "https://example.test/run/1", + "description": "smoke job failed", + } + ], + } + counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False) + # The bug-class assertion: pre-rev4 this would have been 0, with + # preserved_non_failure=1. Rev4 reads `status` → routes to compensate. + assert counters["compensated"] == 1, ( + "Compensation path unreachable: status-reaper still reads `state` " + "instead of `status` on per-entry combined.statuses[] items " + "(rev1-3 bug)." + ) + assert counters["preserved_non_failure"] == 0 + assert len(calls) == 1 + assert calls[0][0] == "POST" + assert calls[0][1] == f"/repos/owner/repo/statuses/{SHA}" + + +def test_reap_per_context_status_key_takes_precedence_over_state( + sr_module, monkeypatch +): + """Defensive: if both `status` and `state` are present (e.g. a + hypothetical Gitea version emits both), `status` (the canonical + Gitea 1.22.6 key) wins. Guards against a future regression where + a fixture or future Gitea release emits stale `state="success"` + while `status="failure"` is the truth.""" + 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 = {"staging-smoke": False} + combined = { + "state": "failure", + "statuses": [ + { + "context": "staging-smoke / smoke (push)", + # Both keys present — vendor-truth `status` MUST win. + "status": "failure", + "state": "success", + "target_url": "https://example.test/run/2", + "description": "smoke job failed", + } + ], + } + counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False) + assert counters["compensated"] == 1 + assert counters["preserved_non_failure"] == 0 + assert len(calls) == 1 + + +def test_reap_per_context_state_only_fallback(sr_module, monkeypatch): + """Backward-compat: a test fixture or older Gitea variant that emits + only `state` (no `status`) must still flow through compensation. + Belt-and-suspenders against future fixture drift. Keeps rev1-3 + `state`-using fixtures green.""" + 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 = {"staging-smoke": False} + combined = { + "state": "failure", + "statuses": [ + { + "context": "staging-smoke / smoke (push)", + "state": "failure", # legacy fixture shape only + "target_url": "https://example.test/run/3", + } + ], + } + counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False) + assert counters["compensated"] == 1 + assert len(calls) == 1 + + +def test_reap_per_context_missing_both_keys_preserves(sr_module, monkeypatch): + """A per-entry item lacking BOTH `status` and `state` must be + preserved (counted under preserved_non_failure). This is the only + correctly-behaving leg of the pre-rev4 bug — exercising it ensures + the fallback chain doesn't accidentally over-compensate on + malformed entries.""" + monkeypatch.setattr( + sr_module, "api", + lambda *a, **kw: (_ for _ in ()).throw( + AssertionError("api should not be called") + ), + ) + + workflow_map = {"staging-smoke": False} + combined = { + "state": "failure", + "statuses": [ + { + "context": "staging-smoke / smoke (push)", + # No status, no state — neither key present. + "target_url": "https://example.test/run/4", + } + ], + } + counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False) + assert counters["compensated"] == 0 + assert counters["preserved_non_failure"] == 1 + + # -------------------------------------------------------------------------- # ApiError propagation # --------------------------------------------------------------------------