fix: make gateway reliability safe by default (#3438)

src/praisonai-bot/praisonai_bot/bots/_reliability.py

@@ -3,24 +3,44 @@
 
 The gateway ships strong reliability building blocks — a durable inbound
 journal (default-on at the session level), a durable outbound outbox, gateway
--wide admission control, and graceful-shutdown draining — but the two strongest
-lifecycle knobs (graceful drain and inbound admission) are individually opt-in.
-An operator running the gateway the "obvious" way therefore silently gets a
-no-backpressure deployment that cuts in-flight turns on restart.
+-wide admission control, and graceful-shutdown draining. Historically the two
+strongest lifecycle knobs (graceful drain and inbound admission) were
+individually opt-in, so an operator running the gateway the "obvious" way
+silently got a no-backpressure deployment that cut in-flight turns on restart.
+As of Issue #3438 the *unset* posture is safe by default (bounded admission +
+graceful drain, bind-aware); ``reliability="off"`` opts back into the old
+immediate-teardown behaviour.
 
 This module resolves a single, discoverable ``reliability`` preset onto the
 already-existing :class:`BotOS` constructor arguments so the happy path is
 production-grade in one switch, while explicit fields still win.
 
+Safe by default (Issue #3438)
+-----------------------------
+Leaving ``reliability`` unset (``None``) now resolves to a *safe* posture
+instead of the old no-backpressure one: a bounded admission ceiling + fair
+wait queue and a graceful-drain window, so an operator running the gateway
+the "obvious" way gets backpressure and does not cut in-flight turns on a
+restart. The posture is bind-aware — a gateway bound to a non-loopback
+interface (an actual deployment) resolves to the full ``production`` window,
+while a loopback bind keeps the same admission ceiling with a snappier drain.
+Opting back into today's immediate-teardown behaviour is an explicit
+``reliability="off"``.
+
 Profiles
 --------
 ``"production"``
     Graceful drain (15s window), inbound admission with a CPU-scaled
     concurrency ceiling and a bounded fair wait queue.
-``"default"`` / ``None``
-    A sane, small graceful-drain window (5s) so a restart doesn't cut
-    in-flight turns, but no admission ceiling (unbounded, legacy dispatch).
-    Durable inbound journal remains on by default (session level).
+``None`` (unset)
+    Safe by default: bounded admission ceiling + fair wait queue plus a
+    graceful-drain window, bind-aware (full ``production`` window when
+    externally bound, a snappy 5s drain on loopback).
+``"default"``
+    The explicit legacy posture: a sane, small graceful-drain window (5s) so
+    a restart doesn't cut in-flight turns, but no admission ceiling
+    (unbounded, legacy dispatch). Durable inbound journal remains on by
+    default (session level).
 ``"off"``
     Today's immediate-teardown behaviour: no drain, no admission.
 
@@ -33,6 +53,7 @@
 
 from __future__ import annotations
 
+import ipaddress
 import os
 from dataclasses import dataclass
 from typing import Optional
@@ -51,6 +72,37 @@
 
 _KNOWN_PROFILES = ("production", "default", "off")
 
+# Non-IP hostnames that mean "not an actual external deployment". A bind to
+# any of these (or to any loopback IP, detected numerically below) keeps the
+# safe-by-default admission ceiling but with a snappy drain; anything else (a
+# real interface) resolves to the full ``production`` window (Issue #3438).
+_LOOPBACK_HOSTNAMES = frozenset({"", "localhost", "loopback"})
+
+
+def _is_externally_bound(bind_host: Optional[str]) -> bool:
+    """Whether *bind_host* looks like a real (non-loopback) deployment bind.
+
+    ``0.0.0.0`` / ``::`` (bind-all) and any concrete non-loopback address count
+    as external; ``None`` (unknown) is treated as loopback so we never guess a
+    host is external without evidence. Loopback is detected numerically via
+    :mod:`ipaddress`, so every valid loopback form — ``127.0.0.2``,
+    ``127.255.255.255``, an expanded ``0:0:0:0:0:0:0:1`` — is recognised, not
+    just the canonical ``127.0.0.1`` / ``::1`` spellings.
+    """
+    if bind_host is None:
+        return False
+    host = str(bind_host).strip().lower()
+    if host in _LOOPBACK_HOSTNAMES:
+        return False
+    # Strip an IPv6 zone id / brackets, then classify numerically. A bare
+    # hostname that isn't a literal IP (e.g. a DNS name) is treated as an
+    # external bind — we only special-case the known loopback names above.
+    candidate = host.strip("[]").split("%", 1)[0]
+    try:
+        return not ipaddress.ip_address(candidate).is_loopback
+    except ValueError:
+        return True
+
 
 def _cpu_scaled_ceiling() -> int:
     """A conservative CPU-scaled default concurrency ceiling.
@@ -112,6 +164,7 @@ def normalize_reliability(reliability: Optional[str]) -> Optional[str]:
 def resolve_reliability(
     reliability: Optional[str],
     *,
+    bind_host: Optional[str] = None,
     drain_timeout: Optional[float] = None,
     max_concurrent_runs: int = 0,
     queue_depth: int = 0,
@@ -123,7 +176,11 @@ def resolve_reliability(
 
     Args:
         reliability: Profile name (``"production"`` | ``"default"`` | ``"off"``)
-            or ``None`` for the default posture.
+            or ``None`` for the safe-by-default posture (Issue #3438).
+        bind_host: The host the gateway binds to; informs the *unset* default
+            posture — a non-loopback bind (an actual deployment) resolves to
+            the full ``production`` window, loopback keeps a snappy drain.
+            Ignored once an explicit preset is chosen.
         drain_timeout: Explicit graceful-drain window; ``None`` means "let the
             preset decide".
         max_concurrent_runs: Explicit admission ceiling; a positive value wins
@@ -141,6 +198,15 @@ def resolve_reliability(
     """
     profile = normalize_reliability(reliability)
 
+    # Safe by default (Issue #3438): an unset posture (``None``) resolves to a
+    # backpressured deployment rather than the old no-admission one. A real
+    # (non-loopback) bind becomes the full ``production`` posture; loopback
+    # keeps the same admission ceiling with a snappier drain. An explicit
+    # ``"default"`` / ``"off"`` / ``"production"`` is respected as-is.
+    unset_default = profile is None
+    if unset_default:
+        profile = "production" if _is_externally_bound(bind_host) else "__safe__"
+
     # Start from the caller's explicit values (these always win).
     resolved_drain = drain_timeout
     resolved_max = int(max_concurrent_runs or 0)
@@ -157,10 +223,11 @@ def resolve_reliability(
             f"outbound_ordering must be 'strict' or 'best_effort', "
             f"got {outbound_ordering!r}"
         )
-    # An explicit ordering always wins; otherwise only production upgrades to
-    # strict, keeping default/off backward compatible.
+    # An explicit ordering always wins; otherwise the production posture and
+    # the safe-by-default posture upgrade to strict per-lane FIFO, keeping the
+    # explicit ``default``/``off`` presets backward compatible (best-effort).
     resolved_ordering = outbound_ordering or (
-        "strict" if profile == "production" else "best_effort"
+        "strict" if profile in ("production", "__safe__") else "best_effort"
     )
 
     if profile == "off":
@@ -176,9 +243,17 @@ def resolve_reliability(
             outbound_ordering=resolved_ordering,
         )
 
-    if profile == "production":
+    if profile in ("production", "__safe__"):
+        # ``production`` and the loopback safe-default share the same admission
+        # ceiling + bounded fair queue; they differ only in the drain window
+        # (a real deployment gets the longer production window, loopback keeps
+        # a snappy restart).
         if resolved_drain is None:
-            resolved_drain = _PRODUCTION_DRAIN_SECONDS
+            resolved_drain = (
+                _PRODUCTION_DRAIN_SECONDS
+                if profile == "production"
+                else _DEFAULT_DRAIN_SECONDS
+            )
         if not explicit_admission:
             resolved_max = _cpu_scaled_ceiling()
             if resolved_queue <= 0:
@@ -195,8 +270,8 @@ def resolve_reliability(
             outbound_ordering=resolved_ordering,
         )
 
-    # profile in (None, "default"): a sane small drain window so a restart does
-    # not cut in-flight turns, but no admission ceiling by default.
+    # profile == "default" (explicit legacy posture): a sane small drain window
+    # so a restart does not cut in-flight turns, but no admission ceiling.
     if resolved_drain is None:
         resolved_drain = _DEFAULT_DRAIN_SECONDS
     return ResolvedReliability(

src/praisonai-bot/praisonai_bot/cli/features/gateway.py

@@ -398,34 +398,38 @@ def _commit_start_flags() -> None:
         # CLI admission-control flags also apply in no-config mode (#2454):
         # build a shared gate directly so `--max-concurrent-runs` is honoured
         # even without a gateway.yaml. A ``--reliability`` preset (#2531) can
-        # supply the admission ceiling too, so build the gate whenever either
-        # is given, letting the preset fill fields the explicit flags omit.
-        if max_concurrent_runs is not None or reliability is not None:
-            try:
-                from praisonai_bot.bots._admission import build_admission_gate
-                from praisonai_bot.bots._reliability import resolve_reliability
-
-                # Pass the explicit ``--drain-timeout`` through so it still wins
-                # over the preset; capture the resolved window for shutdown so
-                # ``--reliability production`` actually drains (#2531).
-                _resolved = resolve_reliability(
-                    reliability,
-                    drain_timeout=drain_timeout,
-                    max_concurrent_runs=max_concurrent_runs or 0,
-                    queue_depth=queue_depth or 0,
-                    overflow_policy=overflow_policy or "reject",
-                )
-                resolved_drain_timeout = _resolved.drain_timeout
-                self._gateway._admission_gate = build_admission_gate(
-                    max_concurrent_runs=_resolved.max_concurrent_runs,
-                    queue_depth=_resolved.queue_depth,
-                    overflow_policy=_resolved.overflow_policy,
-                )
-            except Exception as e:
-                # Invalid admission-control config is unrecoverable until the
-                # operator fixes it; restarting won't help (#2437).
-                print(f"Error: invalid admission-control config: {e}")
-                return GATEWAY_FATAL_CONFIG_EXIT_CODE
+        # supply the admission ceiling too. As of #3438 the *unset* posture is
+        # safe by default (bind-aware admission ceiling + drain), so we always
+        # resolve reliability here — the resolver returns an admission ceiling
+        # unless the operator passes ``--reliability off``.
+        try:
+            from praisonai_bot.bots._admission import build_admission_gate
+            from praisonai_bot.bots._reliability import resolve_reliability
+
+            # Pass the explicit ``--drain-timeout`` through so it still wins
+            # over the preset; capture the resolved window for shutdown so
+            # ``--reliability production`` actually drains (#2531). ``host``
+            # informs the safe-by-default posture — a non-loopback bind
+            # resolves to the full production window (#3438).
+            _resolved = resolve_reliability(
+                reliability,
+                bind_host=host,
+                drain_timeout=drain_timeout,
+                max_concurrent_runs=max_concurrent_runs or 0,
+                queue_depth=queue_depth or 0,
+                overflow_policy=overflow_policy or "reject",
+            )
+            resolved_drain_timeout = _resolved.drain_timeout
+            self._gateway._admission_gate = build_admission_gate(
+                max_concurrent_runs=_resolved.max_concurrent_runs,
+                queue_depth=_resolved.queue_depth,
+                overflow_policy=_resolved.overflow_policy,
+            )
+        except Exception as e:
+            # Invalid admission-control config is unrecoverable until the
+            # operator fixes it; restarting won't help (#2437).
+            print(f"Error: invalid admission-control config: {e}")
+            return GATEWAY_FATAL_CONFIG_EXIT_CODE
 
 
         if agent_file:

src/praisonai-bot/tests/unit/bots/test_outbox_ordering.py

@@ -148,8 +148,14 @@ def test_reliability_production_selects_strict():
     assert r.outbound_ordering == "strict"
 
 
-def test_reliability_default_and_off_stay_best_effort():
-    assert resolve_reliability(None).outbound_ordering == "best_effort"
+def test_reliability_unset_default_is_strict():
+    # Safe by default (#3438): the unset posture upgrades to strict per-lane
+    # FIFO along with its admission ceiling.
+    assert resolve_reliability(None).outbound_ordering == "strict"
+
+
+def test_reliability_explicit_default_and_off_stay_best_effort():
+    # The explicit legacy presets remain backward-compatible (best-effort).
     assert resolve_reliability("default").outbound_ordering == "best_effort"
     assert resolve_reliability("off").outbound_ordering == "best_effort"
 

src/praisonai-bot/tests/unit/bots/test_reliability.py

@@ -11,16 +11,49 @@
 )
 
 
-def test_default_posture_applies_small_drain_no_admission():
-    """Unset reliability gives a sane small drain window but no ceiling."""
+def test_unset_posture_is_safe_by_default():
+    """Unset reliability is safe by default: admission ceiling + drain (#3438)."""
     r = resolve_reliability(None)
+    # Snappy drain on the (unknown → loopback) bind, but a real admission
+    # ceiling and bounded fair queue so a burst can't fan out unboundedly.
     assert r.drain_timeout == 5.0
-    assert r.max_concurrent_runs == 0
-    assert r.queue_depth == 0
+    assert r.max_concurrent_runs > 0
+    assert r.queue_depth > 0
+    assert r.overflow_policy == "queue"
+
+
+def test_unset_externally_bound_is_full_production():
+    """An unset posture on a non-loopback bind resolves to production (#3438)."""
+    r = resolve_reliability(None, bind_host="0.0.0.0")
+    assert r.drain_timeout == 15.0
+    assert r.max_concurrent_runs > 0
+    assert r.queue_depth > 0
+    assert r.overflow_policy == "queue"
+    assert r.outbound_ordering == "strict"
+
+
+def test_unset_loopback_bind_stays_snappy():
+    """Loopback binds keep the ceiling but a snappy drain window (#3438)."""
+    for host in ("127.0.0.1", "localhost", "::1", None):
+        r = resolve_reliability(None, bind_host=host)
+        assert r.drain_timeout == 5.0
+        assert r.max_concurrent_runs > 0
+
 
+def test_unset_noncanonical_loopback_bind_stays_snappy():
+    """Any valid loopback form is recognised, not just 127.0.0.1/::1 (#3438)."""
+    for host in ("127.0.0.2", "127.255.255.255", "0:0:0:0:0:0:0:1", "[::1]"):
+        r = resolve_reliability(None, bind_host=host)
+        assert r.drain_timeout == 5.0, host
+        assert r.max_concurrent_runs > 0, host
 
-def test_default_alias_matches_none():
-    assert resolve_reliability("default") == resolve_reliability(None)
+
+def test_explicit_default_is_legacy_no_admission():
+    """Explicit reliability='default' keeps the legacy no-ceiling posture."""
+    r = resolve_reliability("default")
+    assert r.drain_timeout == 5.0
+    assert r.max_concurrent_runs == 0
+    assert r.queue_depth == 0
 
 
 def test_production_enables_drain_admission_bounded_queue():
@@ -101,6 +134,16 @@ def test_botos_reliability_off_no_drain_no_gate():
     assert os_._admission_gate is None
 
 
+def test_botos_unset_reliability_is_safe_by_default():
+    """BotOS() with no reliability arg is backpressured by default (#3438)."""
+    from praisonai_bot.bots.botos import BotOS
+
+    os_ = BotOS(bots=[])
+    assert os_._drain_timeout == 5.0
+    assert os_._admission_gate is not None
+    assert os_._admission_gate.enabled
+
+
 def test_botos_explicit_drain_overrides_reliability():
     from praisonai_bot.bots.botos import BotOS
 
@@ -159,3 +202,13 @@ def test_cli_no_config_explicit_drain_overrides_reliability():
 def test_cli_no_config_reliability_off_immediate_teardown():
     """`--reliability off` (no config) tears down immediately (drain 0)."""
     assert _run_no_config_gateway_start(reliability="off") == 0.0
+
+
+def test_cli_no_config_unset_loopback_safe_default_drains():
+    """No reliability + loopback bind (no config) drains with the safe window."""
+    assert _run_no_config_gateway_start(host="127.0.0.1") == 5.0
+
+
+def test_cli_no_config_unset_external_bind_full_production_drain():
+    """No reliability + a non-loopback bind gets the full production drain."""
+    assert _run_no_config_gateway_start(host="0.0.0.0") == 15.0