fix(bot): bound gateway subprocess calls so a hung command cannot stall the loop

Four gateway paths shelled out with no timeout. The dispatcher calls them
from inside the asyncio event loop, so a child blocked on stdin — a git
credential prompt, an index.lock, a confirmation prompt — stopped the whole
gateway serving, indefinitely and silently.

Adds praisonai_bot/_guarded_subprocess.run_guarded: a drop-in wrapper that
always bounds the call and, on timeout, returns a CompletedProcess with
returncode 124 and a *diagnosis* in stderr rather than raising. Existing
`returncode == 0` call sites therefore keep working without growing an
exception path.

Two defences, in order:

- stdin is /dev/null and GIT_TERMINAL_PROMPT=0 / GCM_INTERACTIVE=never /
  DEBIAN_FRONTEND=noninteractive are layered under the caller's env, so the
  common cases fail fast instead of blocking. The timeout is a backstop.
- On timeout the partial output is matched against known stall signatures
  and the caller gets an actionable line — "credential prompt", "git lock
  contention", "confirmation prompt, re-run with --yes", "SSH host-key
  prompt" — instead of a bare elapsed time.

Adopted at the four sites that can block the event loop:

- gateway/kanban_dispatcher.py::_run_git — the worst of them, reached from
  dispatch_once and the worktree integration path
- kanban/sqlite_store.py — git rev-parse on the create_task path
- gateway/preflight.py — launchctl on the startup path
- gateway/pairing.py — icacls while hardening the pairing secret

Git keeps a deliberately generous 300s ceiling (PRAISONAI_KANBAN_GIT_TIMEOUT)
because worktree add and merge on a large repository are legitimately slow;
the aim is to bound an indefinite hang, not to fail slow-but-healthy work.
Other commands use 120s (PRAISONAI_SUBPROCESS_TIMEOUT).

Scope is deliberately narrow. The ~66 remaining subprocess.run sites without
timeouts are in daemon/ and CLI paths, where a hang stalls one user command
rather than the gateway; those are worth a separate pass.

Tests: 32 new, covering timeout-without-raising, stdin EOF, each diagnosis
signature, env layering and precedence, unusable env overrides, and that
_run_git routes through the guard.

Verified: tests/unit/kanban + tests/unit/gateway + the new file — 625 passed,
22 failed. Those 22 are identical to the pre-change baseline on this tree
(confirmed by stashing); two further files fail to collect because the
optional `websockets` package is absent here. Neither is related to this
change.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_014ZfTXTzdudKb4oswNSHpHw

src/praisonai-bot/praisonai_bot/_guarded_subprocess.py

@@ -0,0 +1,174 @@
+"""Subprocess execution that cannot hang the caller indefinitely.
+
+Several gateway paths shell out while an asyncio event loop is waiting on
+them. ``subprocess.run`` without ``timeout=`` blocks forever if the child
+waits on stdin -- a git credential prompt, an ``index.lock`` contention, a
+confirmation prompt -- and the whole gateway stops serving.
+
+This module provides :func:`run_guarded`, a drop-in replacement that always
+bounds the call and, when it does fire, hands the caller a *diagnosis* rather
+than a bare timeout. Knowing that a command died waiting on a confirmation
+prompt is actionable; knowing only that 120 seconds elapsed is not.
+
+Note on semantics: this is a total-runtime cap, not an inactivity timer. A
+command that streams output steadily for longer than the cap is still killed.
+Callers with legitimately long work should pass an explicit ``timeout``.
+"""
+
+from __future__ import annotations
+
+import os
+import subprocess
+from typing import Mapping, Optional, Sequence
+
+__all__ = ["run_guarded", "DEFAULT_TIMEOUT", "diagnose"]
+
+#: Total seconds a guarded command may run before it is killed.
+#: Override per call, or globally via ``PRAISONAI_SUBPROCESS_TIMEOUT``.
+DEFAULT_TIMEOUT = 120.0
+
+#: Returncode reported for a killed command. 124 is the conventional exit
+#: status used by coreutils ``timeout``.
+TIMEOUT_RETURNCODE = 124
+
+_ENV_TIMEOUT = "PRAISONAI_SUBPROCESS_TIMEOUT"
+
+# Substrings that identify why a child process is sitting idle. Ordered most
+# specific first; the first hit wins.
+_SIGNATURES: tuple[tuple[tuple[str, ...], str], ...] = (
+    (
+        ("username for", "password for", "passphrase", "authentication failed"),
+        "DETECTED: credential prompt. The command is waiting for a login that "
+        "will never arrive. Configure a credential helper, or set "
+        "GIT_TERMINAL_PROMPT=0 so it fails fast instead of blocking.",
+    ),
+    (
+        ("index.lock", "unable to create", "another git process"),
+        "DETECTED: git lock contention. Another git process holds the "
+        "repository lock. Retry once it exits, or remove a stale .git/index.lock.",
+    ),
+    (
+        ("[y/n]", "(yes/no)", "are you sure", "continue?", "overwrite?", "proceed?"),
+        "DETECTED: confirmation prompt. Re-run non-interactively with the "
+        "command's own flag (--yes, -y, --force, --no-confirm).",
+    ),
+    (
+        ("host key", "fingerprint", "known_hosts"),
+        "DETECTED: SSH host-key prompt. Pre-populate known_hosts, or pass "
+        "-o StrictHostKeyChecking=accept-new.",
+    ),
+)
+
+_GENERIC = (
+    "No interactive prompt was detected. The command may be genuinely slow, "
+    "blocked on network I/O, or deadlocked. Raise timeout= if the work is "
+    "expected to take longer."
+)
+
+
+def diagnose(captured: str) -> str:
+    """Return a human- and model-actionable reason a command stalled.
+
+    ``captured`` is whatever the child emitted before it was killed; it may be
+    empty, which is itself common for a process blocked on stdin.
+    """
+    haystack = (captured or "").lower()
+    for needles, message in _SIGNATURES:
+        if any(needle in haystack for needle in needles):
+            return message
+    return _GENERIC
+
+
+def _default_timeout() -> float:
+    """Resolve the global timeout, ignoring an unusable env override."""
+    raw = os.environ.get(_ENV_TIMEOUT)
+    if not raw:
+        return DEFAULT_TIMEOUT
+    try:
+        value = float(raw)
+    except (TypeError, ValueError):
+        return DEFAULT_TIMEOUT
+    return value if value > 0 else DEFAULT_TIMEOUT
+
+
+def _non_interactive_env(env: Optional[Mapping[str, str]]) -> dict:
+    """Environment that makes common tools fail rather than prompt.
+
+    Suppressing the prompt at source is better than killing the process after
+    it hangs: the command returns an error the caller can act on immediately.
+    """
+    resolved = dict(env if env is not None else os.environ)
+    resolved.setdefault("GIT_TERMINAL_PROMPT", "0")
+    resolved.setdefault("GCM_INTERACTIVE", "never")
+    resolved.setdefault("DEBIAN_FRONTEND", "noninteractive")
+    return resolved
+
+
+def run_guarded(
+    cmd: Sequence[str],
+    *,
+    timeout: Optional[float] = None,
+    cwd: Optional[str] = None,
+    env: Optional[Mapping[str, str]] = None,
+    interactive: bool = False,
+    **kwargs,
+) -> subprocess.CompletedProcess:
+    """Run ``cmd`` with a bounded runtime, never raising on timeout.
+
+    Behaves like ``subprocess.run(..., capture_output=True, text=True)``. On
+    timeout the child is killed and a :class:`subprocess.CompletedProcess` is
+    returned with returncode :data:`TIMEOUT_RETURNCODE` and a diagnosis
+    appended to stderr, so existing ``returncode == 0`` checks keep working
+    instead of having to grow a new exception path.
+
+    Args:
+        cmd: Command and arguments.
+        timeout: Total seconds allowed. Defaults to
+            ``PRAISONAI_SUBPROCESS_TIMEOUT`` or :data:`DEFAULT_TIMEOUT`.
+        cwd: Working directory.
+        env: Environment; prompt-suppressing defaults are layered underneath.
+        interactive: Set True only for a command that must reach a terminal.
+            Skips prompt suppression; the timeout still applies.
+        **kwargs: Forwarded to ``subprocess.run``.
+    """
+    resolved_timeout = _default_timeout() if timeout is None else timeout
+    kwargs.setdefault("capture_output", True)
+    kwargs.setdefault("text", True)
+
+    # stdin at /dev/null turns "wait forever for input" into an immediate EOF
+    # for tools that honour it, so the timeout is a backstop rather than the
+    # primary defence.
+    if not interactive:
+        kwargs.setdefault("stdin", subprocess.DEVNULL)
+
+    try:
+        return subprocess.run(
+            list(cmd),
+            timeout=resolved_timeout,
+            cwd=cwd,
+            env=env if interactive else _non_interactive_env(env),
+            **kwargs,
+        )
+    except subprocess.TimeoutExpired as exc:
+        partial_out = _as_text(exc.stdout)
+        partial_err = _as_text(exc.stderr)
+        reason = diagnose(f"{partial_out}\n{partial_err}")
+        detail = (
+            f"Command killed after {resolved_timeout:g}s: {' '.join(cmd)}\n"
+            f"{reason}"
+        )
+        return subprocess.CompletedProcess(
+            args=list(cmd),
+            returncode=TIMEOUT_RETURNCODE,
+            stdout=partial_out,
+            stderr=(partial_err + "\n" + detail).strip(),
+        )
+
+
+def _as_text(value) -> str:
+    """Normalise TimeoutExpired's stdout/stderr, which may be bytes or None."""
+    if value is None:
+        return ""
+    if isinstance(value, bytes):
+        return value.decode("utf-8", errors="replace")
+    return str(value)

src/praisonai-bot/praisonai_bot/gateway/kanban_dispatcher.py

@@ -14,8 +14,28 @@ import subprocess
 import time
 from typing import Dict, Any, List, Optional
 
+from .._guarded_subprocess import run_guarded
+
 logger = logging.getLogger(__name__)
 
+# Ceiling for a single git invocation. Worktree add, merge and commit on a
+# large repository are legitimately slow, so this is set well above the
+# general default: the aim is to bound an indefinite hang, not to fail slow
+# work. Override with PRAISONAI_KANBAN_GIT_TIMEOUT (seconds).
+_DEFAULT_GIT_TIMEOUT = 300.0
+
+
+def _git_timeout() -> float:
+    """Resolve the per-git-command ceiling, ignoring an unusable override."""
+    raw = os.environ.get("PRAISONAI_KANBAN_GIT_TIMEOUT")
+    if not raw:
+        return _DEFAULT_GIT_TIMEOUT
+    try:
+        value = float(raw)
+    except (TypeError, ValueError):
+        return _DEFAULT_GIT_TIMEOUT
+    return value if value > 0 else _DEFAULT_GIT_TIMEOUT
+
 # Global dispatcher state
 _dispatcher_running = False
 _dispatcher_task = None
@@ -415,12 +435,22 @@ class KanbanDispatcher:
 
         A thin subprocess wrapper so worktree isolation stays self-contained in
         the wrapper without pulling in a Tier-2 package dependency.
+
+        Bounded via :func:`run_guarded`: the dispatcher calls this from inside
+        the gateway's event loop, so a git command blocked on a credential
+        prompt or an ``index.lock`` would otherwise stall every WebSocket
+        client indefinitely. On timeout the result carries returncode 124 and
+        a diagnosis in stderr, which the existing ``returncode == 0`` checks
+        already treat as failure.
+
+        The ceiling is deliberately generous — worktree creation and merges on
+        a large repository are legitimately slow, and the goal is to bound a
+        hang, not to fail slow-but-healthy work.
         """
-        return subprocess.run(
+        return run_guarded(
             ["git", *args],
-            capture_output=True,
-            text=True,
             cwd=cwd or self._repo_dir(),
+            timeout=_git_timeout(),
         )
 
     def _worktree_root(self) -> str:

src/praisonai-bot/praisonai_bot/gateway/pairing.py

@@ -111,7 +111,8 @@ def _restrict_windows_acl(secret_path: str) -> bool:
     """
     try:
         import getpass
-        import subprocess
+
+        from .._guarded_subprocess import run_guarded
 
         user = os.environ.get("USERNAME") or getpass.getuser()
         # Resolve icacls from the trusted system directory rather than PATH
@@ -123,7 +124,9 @@ def _restrict_windows_acl(secret_path: str) -> bool:
         )
         # Quote the username so principals containing spaces (e.g. "John Doe")
         # are parsed as a single principal by icacls.
-        result = subprocess.run(
+        # Bounded: this runs while hardening the pairing secret on the gateway
+        # startup path, so a stalled icacls must not block it forever.
+        result = run_guarded(
             [
                 icacls,
                 secret_path,
@@ -131,7 +134,7 @@ def _restrict_windows_acl(secret_path: str) -> bool:
                 "/grant:r",
                 f'"{user}":F',
             ],
-            capture_output=True,
+            timeout=15,
             check=False,
         )
         return result.returncode == 0

src/praisonai-bot/praisonai_bot/gateway/preflight.py

@@ -879,7 +879,8 @@ def _read_env_file_tokens(path: str) -> Dict[str, str]:
 def _scan_launch_agent(label: str) -> DuplicateService:
     """Inspect a macOS LaunchAgent plist by label."""
     import plistlib
-    import subprocess
+
+    from .._guarded_subprocess import run_guarded
 
     plist_path = os.path.expanduser(f"~/Library/LaunchAgents/{label}.plist")
     service = DuplicateService(label=label, plist_path=plist_path)
@@ -899,11 +900,9 @@ def _scan_launch_agent(label: str) -> DuplicateService:
             pass
 
     try:
-        result = subprocess.run(
-            ["launchctl", "list", label],
-            capture_output=True,
-            text=True,
-        )
+        # Bounded: preflight runs on the gateway's startup path, and a
+        # launchctl that stalls would hold up service discovery indefinitely.
+        result = run_guarded(["launchctl", "list", label], timeout=15)
         service.running = result.returncode == 0
         if service.running and result.stdout:
             parts = result.stdout.strip().split("\t")

src/praisonai-bot/praisonai_bot/kanban/sqlite_store.py

@@ -223,10 +223,15 @@ class SQLiteKanbanStore:
         if not path or not isinstance(path, str):
             return False
         try:
-            import subprocess
-            result = subprocess.run(
+            from .._guarded_subprocess import run_guarded
+
+            # Bounded: this sits on the create_task path, which the gateway
+            # dispatcher calls from its event loop. A git command that stalls
+            # on a network-mounted or lock-contended path would otherwise
+            # block every card creation.
+            result = run_guarded(
                 ["git", "-C", path, "rev-parse", "--is-inside-work-tree"],
-                capture_output=True, text=True,
+                timeout=15,
             )
             return result.returncode == 0 and result.stdout.strip() == "true"
         except Exception:

src/praisonai-bot/tests/unit/test_guarded_subprocess.py

@@ -0,0 +1,214 @@
+"""Tests for the bounded subprocess wrapper.
+
+The behaviour that matters: a command that would block forever returns, the
+caller can still branch on returncode, and the stderr it gets back says *why*
+the command stalled.
+"""
+
+import os
+import subprocess
+import sys
+
+import pytest
+
+from praisonai_bot._guarded_subprocess import (
+    DEFAULT_TIMEOUT,
+    TIMEOUT_RETURNCODE,
+    diagnose,
+    run_guarded,
+)
+
+
+class TestHappyPath:
+    def test_returns_completed_process(self):
+        result = run_guarded([sys.executable, "-c", "print('ok')"])
+        assert result.returncode == 0
+        assert result.stdout.strip() == "ok"
+
+    def test_propagates_nonzero_exit(self):
+        result = run_guarded([sys.executable, "-c", "import sys; sys.exit(3)"])
+        assert result.returncode == 3
+
+    def test_honours_cwd(self, tmp_path):
+        result = run_guarded(
+            [sys.executable, "-c", "import os; print(os.getcwd())"],
+            cwd=str(tmp_path),
+        )
+        assert os.path.realpath(result.stdout.strip()) == os.path.realpath(str(tmp_path))
+
+
+class TestTimeout:
+    def test_hanging_command_returns_instead_of_blocking(self):
+        result = run_guarded(
+            [sys.executable, "-c", "import time; time.sleep(30)"],
+            timeout=0.5,
+        )
+        assert result.returncode == TIMEOUT_RETURNCODE
+
+    def test_timeout_does_not_raise(self):
+        # The whole point: existing `returncode == 0` call sites keep working
+        # without growing an exception handler.
+        run_guarded([sys.executable, "-c", "import time; time.sleep(30)"], timeout=0.3)
+
+    def test_stderr_names_the_command_and_the_limit(self):
+        result = run_guarded(
+            [sys.executable, "-c", "import time; time.sleep(30)"],
+            timeout=0.4,
+        )
+        assert "killed after 0.4s" in result.stderr
+        assert "time.sleep(30)" in result.stderr
+
+    def test_reading_stdin_is_not_a_hang(self):
+        # stdin is /dev/null, so a command that reads it gets EOF immediately
+        # rather than waiting for the timeout to save it.
+        result = run_guarded(
+            [sys.executable, "-c", "import sys; sys.stdin.read(); print('done')"],
+            timeout=5,
+        )
+        assert result.returncode == 0
+        assert "done" in result.stdout
+
+
+class TestDiagnosis:
+    def test_credential_prompt(self):
+        assert "credential prompt" in diagnose("Username for 'https://github.com':")
+
+    def test_git_lock(self):
+        msg = diagnose("fatal: Unable to create '/repo/.git/index.lock': File exists")
+        assert "lock contention" in msg
+
+    def test_confirmation_prompt(self):
+        assert "confirmation prompt" in diagnose("Overwrite existing file? [y/N]")
+
+    def test_host_key_prompt(self):
+        assert "host-key" in diagnose("The authenticity of host ... key fingerprint is")
+
+    def test_unknown_output_gets_generic_guidance(self):
+        msg = diagnose("compiling module 41 of 900")
+        assert "No interactive prompt was detected" in msg
+
+    def test_empty_output_is_safe(self):
+        # A process blocked on stdin often emits nothing at all.
+        assert diagnose("") == diagnose(None)
+
+    def test_diagnosis_is_attached_on_timeout(self):
+        script = (
+            "import sys, time; "
+            "sys.stderr.write(\"Username for 'https://github.com':\"); "
+            "sys.stderr.flush(); time.sleep(30)"
+        )
+        result = run_guarded([sys.executable, "-c", script], timeout=1.0)
+        assert result.returncode == TIMEOUT_RETURNCODE
+        assert "credential prompt" in result.stderr
+
+
+class TestEnvironment:
+    def test_suppresses_git_prompting_by_default(self):
+        result = run_guarded(
+            [sys.executable, "-c", "import os; print(os.environ['GIT_TERMINAL_PROMPT'])"]
+        )
+        assert result.stdout.strip() == "0"
+
+    def test_caller_env_wins(self):
+        result = run_guarded(
+            [sys.executable, "-c", "import os; print(os.environ['GIT_TERMINAL_PROMPT'])"],
+            env={"GIT_TERMINAL_PROMPT": "1", "PATH": os.environ.get("PATH", "")},
+        )
+        assert result.stdout.strip() == "1"
+
+    def test_interactive_mode_adds_no_suppression(self, monkeypatch):
+        # Clear the ambient value first -- CI images commonly export
+        # GIT_TERMINAL_PROMPT=0 already, which would mask the behaviour under
+        # test. Interactive mode must not *introduce* the variable itself.
+        monkeypatch.delenv("GIT_TERMINAL_PROMPT", raising=False)
+        result = run_guarded(
+            [
+                sys.executable,
+                "-c",
+                "import os; print(os.environ.get('GIT_TERMINAL_PROMPT', 'unset'))",
+            ],
+            interactive=True,
+        )
+        assert result.stdout.strip() == "unset"
+
+    def test_default_mode_suppresses_even_when_ambient_is_unset(self, monkeypatch):
+        monkeypatch.delenv("GIT_TERMINAL_PROMPT", raising=False)
+        result = run_guarded(
+            [sys.executable, "-c", "import os; print(os.environ['GIT_TERMINAL_PROMPT'])"]
+        )
+        assert result.stdout.strip() == "0"
+
+
+class TestTimeoutResolution:
+    def test_env_override(self, monkeypatch):
+        monkeypatch.setenv("PRAISONAI_SUBPROCESS_TIMEOUT", "0.3")
+        result = run_guarded([sys.executable, "-c", "import time; time.sleep(30)"])
+        assert result.returncode == TIMEOUT_RETURNCODE
+
+    @pytest.mark.parametrize("bad", ["", "abc", "0", "-5"])
+    def test_unusable_override_falls_back_to_default(self, monkeypatch, bad):
+        monkeypatch.setenv("PRAISONAI_SUBPROCESS_TIMEOUT", bad)
+        # Must not raise, and must not adopt a non-positive timeout that would
+        # kill every command instantly.
+        result = run_guarded([sys.executable, "-c", "print('fine')"])
+        assert result.returncode == 0
+        assert DEFAULT_TIMEOUT > 0
+
+    def test_explicit_timeout_beats_env(self, monkeypatch):
+        monkeypatch.setenv("PRAISONAI_SUBPROCESS_TIMEOUT", "30")
+        result = run_guarded(
+            [sys.executable, "-c", "import time; time.sleep(30)"], timeout=0.3
+        )
+        assert result.returncode == TIMEOUT_RETURNCODE
+
+
+class TestCompatibility:
+    def test_result_is_a_real_completed_process(self):
+        result = run_guarded([sys.executable, "-c", "import time; time.sleep(5)"], timeout=0.3)
+        assert isinstance(result, subprocess.CompletedProcess)
+        assert result.args == [sys.executable, "-c", "import time; time.sleep(5)"]
+        # Call sites read .stdout/.stderr unconditionally; both must be str.
+        assert isinstance(result.stdout, str)
+        assert isinstance(result.stderr, str)
+
+
+class TestKanbanGitTimeout:
+    """The dispatcher's git ceiling must bound hangs without failing slow work."""
+
+    def test_default_is_generous(self):
+        from praisonai_bot.gateway.kanban_dispatcher import _git_timeout
+
+        # Worktree add / merge on a large repo can legitimately run for
+        # minutes; the bound exists to stop an indefinite hang, not slow work.
+        assert _git_timeout() >= 300.0
+
+    def test_env_override(self, monkeypatch):
+        from praisonai_bot.gateway.kanban_dispatcher import _git_timeout
+
+        monkeypatch.setenv("PRAISONAI_KANBAN_GIT_TIMEOUT", "42")
+        assert _git_timeout() == 42.0
+
+    @pytest.mark.parametrize("bad", ["", "abc", "0", "-1"])
+    def test_unusable_override_falls_back(self, monkeypatch, bad):
+        from praisonai_bot.gateway.kanban_dispatcher import _git_timeout
+
+        monkeypatch.setenv("PRAISONAI_KANBAN_GIT_TIMEOUT", bad)
+        assert _git_timeout() >= 300.0
+
+    def test_run_git_is_bounded(self, monkeypatch):
+        """_run_git must route through run_guarded, not raw subprocess.run."""
+        from praisonai_bot.gateway import kanban_dispatcher as kd
+
+        seen = {}
+
+        def fake_run_guarded(cmd, **kwargs):
+            seen["cmd"] = cmd
+            seen["timeout"] = kwargs.get("timeout")
+            return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="")
+
+        monkeypatch.setattr(kd, "run_guarded", fake_run_guarded)
+        dispatcher = kd.KanbanDispatcher.__new__(kd.KanbanDispatcher)
+        dispatcher._run_git("status")
+
+        assert seen["cmd"][0] == "git"
+        assert seen["timeout"] is not None and seen["timeout"] > 0