From e28b41f7ecf59e983edfc5b209aa9d883bf51ffe Mon Sep 17 00:00:00 2001 From: drew Date: Mon, 18 May 2026 00:21:51 -0400 Subject: [PATCH] =?UTF-8?q?fix(auto-agents):=20bump=20post-push=20CI=20ver?= =?UTF-8?q?ify=20per-call=20budget=2090s=20=E2=86=92=20180s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Observed healthy slow-runner days where 90 s was insufficient: the verifier returned ``pending`` and let bad pushes through to the next dispatcher cycle. 180 s catches the fast-failing checks (lint/format/push-validation typically fail in 30-60 s) AND gives headroom for a backlogged runner where those checks may queue for a minute or two before they actually run. - Per-call budget: 90 → 180 s (doubles polls-per-window from 9 to 18; 10 s poll interval unchanged). - Worst-case cycle math updated in the cycle-budget block comment: 5 PRs × 180 s = 15 min (was 7.5 min) of polling per cycle in the uncapped case. The per-cycle budget (commit 2c43179e7) still caps total polling at IMPLEMENTER_POST_PUSH_CI_CYCLE_BUDGET_S (default 300 s), so the practical worst case is unchanged. - Existing TestPostPushCIVerify docstrings update to reference the new number. Escalation integration tests get an autouse fixture that disables the verifier flag: ``test_implementer_escalation_integration.py`` scripts many ``outcome=resolved`` + head-advanced scenarios but doesn't stub ``fetch_ci_status``, so without the disable the verifier would poll the FakeReviewAPI default for the full 180 s on a real ``time.sleep``. The verifier itself has dedicated coverage in ``TestPostPushCIVerify`` / ``TestPostPushCIVerifyCycleBudget``. uv.lock change for the new ``mcp-servers`` optional dep stays unstaged — it's missing its matching ``pyproject.toml`` entry. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../auto_agents/test_dispatch_implementer.py | 6 +++--- ...test_implementer_escalation_integration.py | 15 ++++++++++++++ tools/dispatch_implementer.py | 20 +++++++++++-------- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/tests/auto_agents/test_dispatch_implementer.py b/tests/auto_agents/test_dispatch_implementer.py index 83720cee8..aa72ac7df 100644 --- a/tests/auto_agents/test_dispatch_implementer.py +++ b/tests/auto_agents/test_dispatch_implementer.py @@ -2459,7 +2459,7 @@ class TestPostPushCIVerify: self, driver, dry_cfg, fake_implementer_api ): """Dry-run must short-circuit so ``--dry-run`` cycles don't - burn 90 s polling a real Forgejo endpoint.""" + burn 180 s polling a real Forgejo endpoint.""" self._stub_post_session_head(fake_implementer_api) stub_forgejo_ci_status( fake_implementer_api, sha="newsha0000feed", state="failure", @@ -2571,8 +2571,8 @@ class TestPostPushCIVerifyCycleBudget: """Per-cycle polling-time budget. Without this, a dispatcher cycle that processes N PRs each landing in ``outcome=resolved`` after a push could spend ``N * IMPLEMENTER_POST_PUSH_CI_VERIFY_S`` - seconds polling — at the default 90 s per call and 5 PRs/cycle, - that's 7.5 min of polling eating the dispatcher cycle. + seconds polling — at the default 180 s per call and 5 PRs/cycle, + that's 15 min of polling eating the dispatcher cycle. The budget auto-resets after ``IMPLEMENTER_POST_PUSH_CI_CYCLE_RESET_AFTER_S`` seconds of idle diff --git a/tests/auto_agents/test_implementer_escalation_integration.py b/tests/auto_agents/test_implementer_escalation_integration.py index b7be02937..3e6383bf7 100644 --- a/tests/auto_agents/test_implementer_escalation_integration.py +++ b/tests/auto_agents/test_implementer_escalation_integration.py @@ -37,6 +37,21 @@ import pytest from .conftest import load_tool_module, make_dispatch_config +@pytest.fixture(autouse=True) +def _disable_post_push_ci_verify(monkeypatch): + """R3.7 (2026-05-17): the post-push CI verifier runs inside + ``_post_session_action_with_escalation`` whenever the worker + claimed ``outcome=resolved`` AND head_sha advanced. This test + file scripts many such scenarios but doesn't stub + ``fetch_ci_status``, so the verifier would poll the FakeReviewAPI + default (``[]``) for the full 90 s budget on a real + ``time.sleep``. Disable the flag here so escalation tests stay + fast and continue testing escalation routing, not the verifier + (which has dedicated coverage in + ``test_dispatch_implementer.py::TestPostPushCIVerify``).""" + monkeypatch.setenv("IMPLEMENTER_POST_PUSH_CI_VERIFY", "0") + + @dataclass class _FakeSession: """Stand-in for ``_opencode_worker.SessionResult`` carrying just diff --git a/tools/dispatch_implementer.py b/tools/dispatch_implementer.py index a697b2404..e001736fa 100644 --- a/tools/dispatch_implementer.py +++ b/tools/dispatch_implementer.py @@ -2661,17 +2661,21 @@ def _fetch_pr_state(cfg: Any, pr_number: int) -> str: # actually pass CI." # # Tuning: -# - Default 90 s: long enough to catch the fast-failing checks -# (lint, format, push-validation typically fail in 30-60 s) but -# short enough not to dominate the dispatcher cycle. -# - 10 s poll interval: 9 polls per budget window. Forgejo's CI +# - Default 180 s: catches the fast-failing checks (lint, format, +# push-validation typically fail in 30-60 s) AND gives headroom +# for a backlogged runner where those same checks may queue for +# a minute or two before they actually run. 90 s was the earlier +# default; bumped after observing healthy slow-runner days where +# the verifier returned ``pending`` and let bad pushes through to +# the next cycle. +# - 10 s poll interval: 18 polls per budget window. Forgejo's CI # status endpoint is fast (< 1 s typical). # - On budget exhaustion the verifier RETURNS the outcome unchanged # (does NOT rewrite to a failure) — we don't penalise the worker # for slow CI. The next cycle will re-classify if CI eventually # fails. _POST_PUSH_CI_VERIFY_BUDGET_S = int( - os.environ.get("IMPLEMENTER_POST_PUSH_CI_VERIFY_S", "90") + os.environ.get("IMPLEMENTER_POST_PUSH_CI_VERIFY_S", "180") ) _POST_PUSH_CI_VERIFY_POLL_INTERVAL_S = int( os.environ.get("IMPLEMENTER_POST_PUSH_CI_VERIFY_POLL_S", "10") @@ -2683,7 +2687,7 @@ _POST_PUSH_CI_TERMINAL_PASS_STATES = frozenset({"success"}) # dispatcher cycle that processes N PRs each landing in # ``outcome=resolved`` after a push could spend # ``N × _POST_PUSH_CI_VERIFY_BUDGET_S`` seconds polling — at the -# default 90 s per call and 5 PRs/cycle, that's 7.5 min of polling +# default 180 s per call and 5 PRs/cycle, that's 15 min of polling # eating the cycle budget. The cycle budget caps the cumulative # polling time across all ``_verify_post_push_ci`` calls in a single # dispatcher cycle. @@ -2733,7 +2737,7 @@ def _post_push_ci_verify_enabled() -> bool: without stubbing ``fetch_ci_status`` set ``"0"`` via an autouse fixture so the verifier short-circuits instead of polling the FakeReviewAPI default (which returns ``[]`` and would burn the - full 90 s budget on a real sleep).""" + full 180 s budget on a real sleep).""" return str( os.environ.get("IMPLEMENTER_POST_PUSH_CI_VERIFY", "1") ).strip().lower() in {"1", "true", "yes", "on"} @@ -2773,7 +2777,7 @@ def _verify_post_push_ci( would create false positives on Forgejo flakes. Dry-run short-circuits to no-op so ``--dry-run`` cycles don't - burn 90 s polling a real Forgejo endpoint. + burn 180 s polling a real Forgejo endpoint. """ if not _post_push_ci_verify_enabled(): return parsed_json