From 4eb17ff7db96b21bdbb46fc82c9db93e923b09a7 Mon Sep 17 00:00:00 2001
From: Renn F <rennf93@users.noreply.github.com>
Date: Mon, 29 Jun 2026 00:25:26 +0200
Subject: [PATCH] [F132] timeout the conventions validator + reap on hang
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

_run_conventions_validator awaited proc.communicate() with no timeout —
a hung subprocess (tree-sitter deadlock, huge repo) hung the
i_am_done/pr_pass gate forever and orphaned the python subprocess on
orchestrator restart. Wrap communicate() in wait_for(120s); on timeout
kill+wait the proc and fail closed (could_not_run=True → block gate
refuses the submit), matching the validator's own fail-loud philosophy.
---
 roboco/services/git.py                        | 23 +++++++++++-
 .../test_git_conventions_check_fail_closed.py | 36 +++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/roboco/services/git.py b/roboco/services/git.py
index b38da26d..29880ce0 100644
--- a/roboco/services/git.py
+++ b/roboco/services/git.py
@@ -181,6 +181,9 @@ _CI_RUN_WINDOW = 20
 _CI_FETCH_ATTEMPTS = 3
 _CI_FETCH_BACKOFF_SECONDS = 0.5
 _CI_RETRYABLE_STATUS = frozenset({429, 500, 502, 503, 504})
+# Cap a conventions-validator run so a hung subprocess (tree-sitter deadlock,
+# huge repo) can't hang the i_am_done/pr_pass gate forever.
+_CONVENTIONS_VALIDATOR_TIMEOUT_SECONDS = 120
 
 
 def _select_ci_head_run(runs: list[dict[str, Any]]) -> dict[str, Any]:
@@ -4347,7 +4350,25 @@ class GitService(BaseService):
             stdout=asyncio.subprocess.PIPE,
             stderr=asyncio.subprocess.PIPE,
         )
-        out, err = await proc.communicate()
+        try:
+            out, err = await asyncio.wait_for(
+                proc.communicate(),
+                timeout=_CONVENTIONS_VALIDATOR_TIMEOUT_SECONDS,
+            )
+        except TimeoutError:
+            # Fail closed (could_not_run=True → block gate refuses the submit),
+            # matching the validator's own fail-loud philosophy, and reap the
+            # killed proc so it isn't orphaned on orchestrator restart.
+            proc.kill()
+            await proc.wait()
+            return {
+                "findings": [],
+                "could_not_run": True,
+                "reason": (
+                    f"validator timed out after "
+                    f"{_CONVENTIONS_VALIDATOR_TIMEOUT_SECONDS}s"
+                ),
+            }
         if proc.returncode != 0:
             reason = err.decode(errors="replace").strip() or "validator crashed"
             return {"findings": [], "could_not_run": True, "reason": reason[:300]}
diff --git a/tests/unit/services/test_git_conventions_check_fail_closed.py b/tests/unit/services/test_git_conventions_check_fail_closed.py
index 2d85878e..05e6a0ce 100644
--- a/tests/unit/services/test_git_conventions_check_fail_closed.py
+++ b/tests/unit/services/test_git_conventions_check_fail_closed.py
@@ -16,11 +16,13 @@ These are empty-result cases, not errors, so the gate correctly passes.
 
 from __future__ import annotations
 
+import asyncio
 from pathlib import Path
 from unittest.mock import AsyncMock, MagicMock
 from uuid import uuid4
 
 import pytest
+from roboco.services import git as git_module
 from roboco.services.git import GitService
 
 
@@ -91,3 +93,37 @@ async def test_no_changed_files_still_fails_open() -> None:
     result = await svc.conventions_check_for_task(uuid4(), _task("feature/backend/abc"))
     assert result["could_not_run"] is False
     assert result["findings"] == []
+
+
+@pytest.mark.asyncio
+async def test_validator_timeout_fails_closed_and_reaps(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """A hung conventions validator subprocess (tree-sitter deadlock, huge repo)
+    must time out, fail closed (could_not_run=True so the block gate refuses the
+    submit), and kill+wait the proc — not hang the gate forever nor orphan the
+    subprocess on orchestrator restart.
+    """
+    fake_proc = MagicMock()
+    fake_proc.returncode = None
+
+    async def _communicate() -> tuple[bytes, bytes]:
+        await asyncio.sleep(30)
+        return (b"", b"")
+
+    fake_proc.communicate = _communicate
+    fake_proc.kill = MagicMock()
+    fake_proc.wait = AsyncMock(return_value=-9)
+
+    async def _fake_exec(*_args: object, **_kwargs: object) -> object:
+        return fake_proc
+
+    monkeypatch.setattr(asyncio, "create_subprocess_exec", _fake_exec)
+    monkeypatch.setattr(git_module, "_CONVENTIONS_VALIDATOR_TIMEOUT_SECONDS", 0.01)
+
+    svc = _service()
+    result = await svc._run_conventions_validator(tmp_path, ["a.py"])
+    assert result["could_not_run"] is True
+    assert "timed out" in (result.get("reason") or "")
+    fake_proc.kill.assert_called_once()
+    fake_proc.wait.assert_awaited_once()