fix: refuse mid-turn play-asset (409) + correct OTA pre-release version ordering

custom-providers/xiaozhi-patches/http_server.py

@@ -438,6 +438,20 @@ async def _dotty_play_asset(self, request: "web.Request") -> "web.Response":
             )
         resolved_id = (getattr(conn, "headers", {}) or {}).get("device-id", "") or device_id
 
+        # DOTTY-PATCH: don't clobber a live chat turn. play-asset is timer-driven
+        # on the first available device, so a purr/song landing mid-conversation
+        # would reset client_abort/client_is_speaking (below) and mark the device
+        # idle in its finally block while chat TTS is still streaming — cancelling
+        # the user's barge-in and corrupting the turn. Refuse instead; a dropped
+        # ambient asset is the correct outcome here.
+        if getattr(conn, "client_is_speaking", False) and not getattr(
+            conn, "client_abort", False
+        ):
+            return web.json_response(
+                {"error": "device busy (mid-turn)", "device_id": resolved_id},
+                status=409,
+            )
+
         async def _dispatch() -> None:
             import json as _json
             import numpy as _np

custom-providers/xiaozhi-patches/ota_handler.py

@@ -21,26 +21,35 @@ def _safe_basename(filename: str) -> str:
     return os.path.basename(filename)
 
 
-def _parse_version(ver: str) -> Tuple[int, ...]:
-    # conservative parser: split by non-digit, keep numeric parts
-    parts = re.findall(r"\d+", ver)
-    return tuple(int(p) for p in parts) if parts else (0,)
+def _parse_version(ver: str) -> Tuple:
+    """Comparison/sort key for a firmware version string.
+
+    Considers only the first three numeric segments (major.minor.patch). A
+    trailing pre-release / build suffix (anything after the numeric core, e.g.
+    ``-rc1``, ``-beta``, ``+build5``) ranks BELOW the corresponding release,
+    matching semver precedence — so a device on GA ``1.2.3`` is never offered a
+    stale ``1.2.3-rc1``. The naive ``re.findall(r"\\d+")`` parser this replaced
+    turned ``1.2.3-rc1`` into ``(1,2,3,1)`` and ranked it ABOVE GA.
+
+    The returned tuple is shaped ``(major, minor, patch, release_rank, suffix)``
+    where release_rank is 1 for a clean release and 0 for any suffixed build,
+    and suffix is compared lexically to order pre-releases deterministically
+    (``-alpha`` < ``-beta``). Element types are positionally consistent, so
+    these keys are safe to compare and sort.
+    """
+    s = ver.strip().lstrip("vV")
+    m = re.match(r"\d+(?:\.\d+)*", s)
+    core = m.group(0) if m else ""
+    nums = [int(p) for p in core.split(".") if p != ""]
+    nums = (nums + [0, 0, 0])[:3]
+    suffix = s[len(core):].lstrip(".-_+ ").strip()
+    release_rank = 1 if suffix == "" else 0
+    return (nums[0], nums[1], nums[2], release_rank, suffix)
 
 
 def _is_higher_version(a: str, b: str) -> bool:
-    """Return True if version string a > b (semver-like numeric compare)."""
-    ta = _parse_version(a)
-    tb = _parse_version(b)
-    # compare tuple lexicographically, but allow different lengths
-    maxlen = max(len(ta), len(tb))
-    for i in range(maxlen):
-        ai = ta[i] if i < len(ta) else 0
-        bi = tb[i] if i < len(tb) else 0
-        if ai > bi:
-            return True
-        if ai < bi:
-            return False
-    return False
+    """Return True if version string a > b (semver-like precedence)."""
+    return _parse_version(a) > _parse_version(b)
 
 
 class OTAHandler(BaseHandler):

tests/test_ota_version.py

@@ -0,0 +1,101 @@
+"""Unit tests for the OTA version comparator (ota_handler.py).
+
+Covers the pre-release / build-suffix precedence bug: the old
+``re.findall(r"\\d+")`` parser turned ``1.2.3-rc1`` into ``(1,2,3,1)`` and
+ranked it ABOVE GA ``1.2.3``, so a device on GA would be offered a stale rc.
+The module's core.* / aiohttp imports are stubbed so the test runs without a
+container; only the pure module-level functions are exercised.
+"""
+import pathlib
+import sys
+import types
+import unittest
+from unittest.mock import MagicMock
+
+# ── Stub the container-only imports ota_handler pulls at load time ────────────
+# NB: do NOT stub `aiohttp` — it's a real installed dep that other test modules
+# in this suite import for real; stubbing it here (setdefault wins on import
+# order) would poison sys.modules and break them.
+for _n in ("core", "core.auth", "core.utils", "core.utils.util", "core.api"):
+    sys.modules.setdefault(_n, MagicMock())
+
+# BaseHandler is used as a real base class (`class OTAHandler(BaseHandler)`), so
+# it must be a genuine type, not a MagicMock attribute.
+_base_mod = types.ModuleType("core.api.base_handler")
+
+
+class _StubBase:
+    def __init__(self, *a, **k):
+        pass
+
+
+_base_mod.BaseHandler = _StubBase  # type: ignore[attr-defined]
+sys.modules["core.api.base_handler"] = _base_mod
+
+# ── Load the module under test by path ───────────────────────────────────────
+import importlib.util as _ilu
+
+_OTA_PY = (
+    pathlib.Path(__file__).parent.parent
+    / "custom-providers"
+    / "xiaozhi-patches"
+    / "ota_handler.py"
+)
+_spec = _ilu.spec_from_file_location("ota_handler_under_test", _OTA_PY)
+_mod = _ilu.module_from_spec(_spec)  # type: ignore[arg-type]
+_spec.loader.exec_module(_mod)  # type: ignore[union-attr]
+
+_is_higher = _mod._is_higher_version
+_key = _mod._parse_version
+
+
+class TestOtaVersion(unittest.TestCase):
+
+    # The headline regression: a pre-release must NOT outrank its GA release.
+    def test_prerelease_not_higher_than_release(self):
+        self.assertFalse(_is_higher("1.2.3-rc1", "1.2.3"))
+        self.assertTrue(_is_higher("1.2.3", "1.2.3-rc1"))
+
+    def test_build_suffix_not_higher_than_release(self):
+        self.assertFalse(_is_higher("1.2.3+build5", "1.2.3"))
+        self.assertTrue(_is_higher("1.2.3", "1.2.3+build5"))
+
+    def test_normal_ordering(self):
+        self.assertTrue(_is_higher("1.2.4", "1.2.3"))
+        self.assertTrue(_is_higher("2.0.0", "1.9.9"))
+        self.assertTrue(_is_higher("1.3.0", "1.2.9"))
+        self.assertFalse(_is_higher("1.2.3", "1.2.4"))
+
+    def test_equal_versions_are_not_higher(self):
+        self.assertFalse(_is_higher("1.2.3", "1.2.3"))
+        # short form equals its zero-padded form (1.2 == 1.2.0)
+        self.assertFalse(_is_higher("1.2", "1.2.0"))
+        self.assertFalse(_is_higher("1.2.0", "1.2"))
+        self.assertEqual(_key("1.2"), _key("1.2.0"))
+
+    def test_leading_v_stripped(self):
+        self.assertFalse(_is_higher("v1.2.3", "1.2.3"))
+        self.assertTrue(_is_higher("v1.2.4", "v1.2.3"))
+
+    def test_prerelease_ordering_is_deterministic(self):
+        # Among pre-releases of the same core, lexical suffix order applies.
+        self.assertTrue(_is_higher("1.0.0-beta", "1.0.0-alpha"))
+        self.assertFalse(_is_higher("1.0.0-alpha", "1.0.0-beta"))
+
+    def test_only_first_three_segments_considered(self):
+        # A 4th numeric segment must not flip precedence vs the 3-segment GA.
+        self.assertFalse(_is_higher("1.2.3.4", "1.2.3"))
+        self.assertEqual(_key("1.2.3.4")[:3], (1, 2, 3))
+
+    def test_key_is_sortable(self):
+        versions = ["1.2.3", "1.2.3-rc1", "1.10.0", "1.2.10", "2.0.0", "v1.2.4"]
+        ordered = sorted(versions, key=_key)
+        # GA 1.2.3 sorts above its rc; numeric (not lexical) segment compare.
+        self.assertLess(ordered.index("1.2.3-rc1"), ordered.index("1.2.3"))
+        self.assertLess(ordered.index("1.2.3"), ordered.index("1.2.10"))
+        self.assertLess(ordered.index("1.2.10"), ordered.index("1.10.0"))
+        self.assertEqual(ordered[-1], "2.0.0")
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_play_asset_route.py

@@ -164,6 +164,20 @@ def test_no_device_returns_503(self):
         self.assertEqual(resp.status, 503)
         self.assertIn("known", _body(resp))
 
+    def test_mid_turn_device_returns_409(self):
+        # Device actively speaking (chat TTS streaming) and not aborted → a
+        # timer-driven asset must not clobber the turn; refuse with 409.
+        path = self._asset()
+        conn = MagicMock()
+        conn.headers = {"device-id": "dev1"}
+        conn.client_is_speaking = True
+        conn.client_abort = False
+        _mock_active["dev1"] = conn
+        req = _FakeRequest(data={"asset": path, "device_id": "dev1"})
+        resp = _run(self.srv._dotty_play_asset(req))
+        self.assertEqual(resp.status, 409)
+        self.assertIn("busy", _body(resp)["error"])
+
     # Happy-path ───────────────────────────────────────────────────────────────
 
     def test_valid_request_fires_task_and_returns_200(self):