kshitijk4poor
c0ff25a1f8
Pattern-A architectural fix: blocking calls inside async functions freeze the gateway/uvicorn event loop for every adapter, timer, and health check. Known incidents: 17-minute getaddrinfo freeze (#91912 class), 10s restart freeze in start_gateway (#36163). Fixes at the four unguarded core sites: - gateway/platforms/webhook.py: `gh pr comment` subprocess (30s timeout) now runs via asyncio.to_thread — a webhook delivery no longer freezes every other platform for the duration of a network call. - gateway/run.py start_gateway --replace: two time.sleep() waits (10s + 5s worst case) become await asyncio.sleep() (re-lands #36163 at current line numbers, credit AhmetArif0). - gateway/slash_commands.py /save: session render + file write move off the loop (scales with transcript size). - hermes_cli/web_server.py voice TTS: multi-MB audio file read + unlink move off the loop. Prevention gate so the bug class cannot re-enter: - pyproject.toml [tool.ruff.lint] select gains ASYNC210/220/221/251 (blocking HTTP / Popen / subprocess.run / time.sleep in async def). These run in the existing blocking `ruff check .` CI job. - Frozen ratchet baseline in per-file-ignores for the remaining legacy sites (detached restart watchers; router sweep in flight via #84376; two platform adapters), each documented for burn-down. New files or new violations fail CI immediately. - tests/** keeps the relaxation (deliberate sleeps in fixtures). Verification: - ruff check . green on this branch; sabotage file with time.sleep + subprocess.run in async def fails the gate with 2 errors. - New behavioral test test_webhook_offloop_delivery.py asserts loop liveness DURING delivery (ticker coroutine): 1 tick on the old blocking code (fails), 21 ticks off-loop (passes). - 50 webhook/replace gateway tests + 26 save/export tests pass. Co-authored-by: AhmetArif0 <147827411+AhmetArif0@users.noreply.github.com>
97 lines
3.5 KiB
Python
97 lines
3.5 KiB
Python
"""Event-loop liveness for webhook GitHub-comment delivery (Pattern A).
|
|
|
|
``_deliver_github_comment`` shells out to ``gh`` with a 30s timeout. Before
|
|
the off-loop fix it called ``subprocess.run`` inline from an ``async def``,
|
|
freezing the entire gateway event loop — every adapter, timer, and health
|
|
check — for up to the full timeout while the network call ran.
|
|
|
|
The contract under test is behavioral, not structural: while delivery is
|
|
awaiting the subprocess, OTHER coroutines on the same loop must keep
|
|
running. A ticker coroutine sampled at the moment delivery returns proves
|
|
it — on the blocking implementation it manages ~1 tick during a 1-second
|
|
``gh`` run; off-loop it manages ~20.
|
|
"""
|
|
|
|
import asyncio
|
|
import os
|
|
import stat
|
|
import sys
|
|
|
|
import pytest
|
|
|
|
from gateway.platforms.webhook import WebhookAdapter
|
|
|
|
|
|
@pytest.fixture()
|
|
def fake_gh(tmp_path, monkeypatch):
|
|
"""A ``gh`` stub that sleeps 1s then succeeds, prepended to PATH."""
|
|
if sys.platform.startswith("win"):
|
|
pytest.skip("POSIX shell stub")
|
|
gh = tmp_path / "gh"
|
|
gh.write_text("#!/bin/bash\nsleep 1\necho posted\nexit 0\n", encoding="utf-8")
|
|
gh.chmod(gh.stat().st_mode | stat.S_IXUSR)
|
|
monkeypatch.setenv("PATH", f"{tmp_path}{os.pathsep}{os.environ['PATH']}")
|
|
return gh
|
|
|
|
|
|
class TestGithubCommentDeliveryOffLoop:
|
|
@pytest.mark.asyncio
|
|
async def test_loop_stays_live_during_gh_subprocess(self, fake_gh):
|
|
"""Concurrent coroutines keep ticking while ``gh`` runs (~1s)."""
|
|
adapter = WebhookAdapter.__new__(WebhookAdapter)
|
|
|
|
ticks = []
|
|
stop = False
|
|
|
|
async def ticker():
|
|
while not stop:
|
|
ticks.append(1)
|
|
await asyncio.sleep(0.05)
|
|
|
|
task = asyncio.create_task(ticker())
|
|
await asyncio.sleep(0) # let the ticker take its first turn
|
|
|
|
result = await adapter._deliver_github_comment(
|
|
"test body",
|
|
{"deliver_extra": {"repo": "owner/repo", "pr_number": "1"}},
|
|
)
|
|
ticks_during_delivery = len(ticks)
|
|
|
|
stop = True
|
|
try:
|
|
await asyncio.wait_for(task, timeout=2)
|
|
except asyncio.TimeoutError:
|
|
task.cancel()
|
|
|
|
assert result.success is True
|
|
# Blocking implementation: ~1 tick (loop frozen for the 1s gh run).
|
|
# Off-loop implementation: ~20 ticks. 10 is a comfortable midpoint
|
|
# that stays robust on slow CI runners.
|
|
assert ticks_during_delivery >= 10, (
|
|
f"event loop starved during gh delivery: only "
|
|
f"{ticks_during_delivery} ticker turns ran in ~1s"
|
|
)
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_delivery_result_faithful_off_loop(self, fake_gh):
|
|
"""Off-loop offload must not change the SendResult contract."""
|
|
adapter = WebhookAdapter.__new__(WebhookAdapter)
|
|
result = await adapter._deliver_github_comment(
|
|
"body",
|
|
{"deliver_extra": {"repo": "owner/repo", "pr_number": "7"}},
|
|
)
|
|
assert result.success is True
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_invalid_inputs_still_rejected_before_subprocess(self):
|
|
"""Validation short-circuits stay synchronous and unchanged."""
|
|
adapter = WebhookAdapter.__new__(WebhookAdapter)
|
|
bad_repo = await adapter._deliver_github_comment(
|
|
"body", {"deliver_extra": {"repo": "not a repo!", "pr_number": "1"}}
|
|
)
|
|
assert bad_repo.success is False
|
|
bad_pr = await adapter._deliver_github_comment(
|
|
"body", {"deliver_extra": {"repo": "owner/repo", "pr_number": "-2"}}
|
|
)
|
|
assert bad_pr.success is False
|