Repository navigation
fix: refuse mid-turn play-asset (409) + correct OTA pre-release version ordering - #143
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses two audit findings in the custom-providers/xiaozhi-patches/ layer: preventing play-asset from interrupting an active chat/TTS turn, and fixing OTA firmware version precedence so pre-releases/build-suffixed versions don’t outrank the corresponding GA release.
Changes:
- Refuse
POST /xiaozhi/admin/play-assetwith HTTP 409 when the target device is mid-turn (speaking and not aborting). - Replace the OTA version parser/comparator with a semver-ish key so
1.2.3-rc1/1.2.3+build5rank below1.2.3, and only the first three numeric segments participate. - Add/extend unit tests covering the new 409 behavior and OTA version ordering/sortability.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
custom-providers/xiaozhi-patches/http_server.py |
Adds a mid-turn “device busy” guard returning 409 to prevent play-asset from clobbering an active chat turn. |
custom-providers/xiaozhi-patches/ota_handler.py |
Updates version parsing/comparison to ensure pre-release/build suffixes don’t outrank GA, and simplifies comparison logic. |
tests/test_play_asset_route.py |
Adds a regression test asserting 409 is returned when play-asset targets a device that is currently speaking. |
tests/test_ota_version.py |
Introduces a focused unit test suite for OTA version precedence, including rc/build handling and sortability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+41
to
+47
| 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) |
Comment on lines
+5
to
+7
| 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. |
…on ordering Two more confirmed audit findings, both in custom-providers/xiaozhi-patches/. play-asset mid-turn clobber (http_server.py): _dispatch() unconditionally reset conn.client_abort=False / client_is_speaking=True with a finally that marks the device idle. Since play-asset is timer-driven on the first available device, a purr/song landing mid-conversation cancelled the user's barge-in and marked the device idle while chat TTS was still streaming. Guard before dispatch: if the device is speaking and not aborting, refuse with 409 (a dropped ambient asset is the correct outcome). OTA version ordering (ota_handler.py): _parse_version used re.findall(r"\d+"), so 1.2.3-rc1 parsed to (1,2,3,1) and ranked ABOVE GA 1.2.3 — a device on GA would be offered a stale rc. Replace with a semver-ish key (major.minor.patch + a release-rank that puts any pre-release/build suffix BELOW the clean release, suffix compared lexically for determinism). Only the first three numeric segments count, so a 4th segment can't flip precedence. Tests: +1 play-asset 409 case; new tests/test_ota_version.py covering rc/build suffixes, leading v, short-form equality, 4-segment truncation, and sortability. Full tests/ suite 61 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BrettKinny
force-pushed
the
audit/playasset-midturn-ota-version
branch
from
June 5, 2026 11:15
649df2d to
96f11fc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two more confirmed-3/3 audit findings, both in
custom-providers/xiaozhi-patches/. Stacked on #142 (base is the #142 branch).play-asset clobbers a live chat turn (
http_server.py)_dispatch()unconditionally resetconn.client_abort=False/client_is_speaking=Truewith a finally that marks the device idle. play-asset is timer-driven on the first available device, so a purr/song landing mid-conversation cancelled the user's barge-in and marked the device idle while chat TTS was still streaming.Fix: before dispatch, if the device is speaking and not aborting, refuse with 409. A dropped ambient asset is the correct outcome.
OTA offers a stale pre-release over GA (
ota_handler.py)_parse_versionusedre.findall(r"\d+"), so1.2.3-rc1→(1,2,3,1)ranked above GA1.2.3— a device on GA would be offered a stale rc. (+buildsuffixes similarly inverted.)Fix: semver-ish key
(major, minor, patch, release_rank, suffix)where any pre-release/build suffix ranks below the clean release, suffix compared lexically for determinism. Only the first three numeric segments count, so a 4th segment can't flip precedence. Used for both the comparison and the existing descending sort.Tests
tests/test_ota_version.py(8 cases): rc/build suffix precedence, leadingv, short-form equality (1.2 == 1.2.0), 4-segment truncation, deterministic pre-release ordering, sortability.tests/suite: 61 passed, ruff clean.🤖 Generated with Claude Code