DeviceCommand seam: one module for server→firmware MCP calls (kills 3 audit bug classes)#160
Open
BrettKinny wants to merge 2 commits into
Open
DeviceCommand seam: one module for server→firmware MCP calls (kills 3 audit bug classes)#160BrettKinny wants to merge 2 commits into
BrettKinny wants to merge 2 commits into
Conversation
…ge labels, domain docs)
Adds docs/agents/{issue-tracker,triage-labels,domain}.md and an
## Agent skills section in CLAUDE.md so the Matt Pocock engineering
skills know: issues live on GitHub (BrettKinny/dotty-stackchan via gh),
the five triage roles map to default label strings (orthogonal to the
existing status:/area: labels), and domain docs are single-context.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Twelve call sites — five /xiaozhi/admin/* handlers in http_server.py
and seven helpers in receiveAudioHandle.py — each hand-rolled the same
MCP JSON-RPC envelope, so every shared defect was twelve defects
(2026-06-06 audit): request ids were int(time.time()*1000)%0x7FFFFFFF
(same-millisecond calls collided), and every site fired
conn.websocket.send() uncoordinated against the other senders on the
same ServerConnection (the websockets library forbids interleaved
sends).
New module custom-providers/xiaozhi-patches/device_command.py, mounted
at core/utils/device_command.py so both patch surfaces import it:
- monotonic per-connection request ids (collision-free for the life of
the connection; stored on the conn so state dies with it)
- the MCP envelope in one place
- per-connection serialized sends via an asyncio.Lock; play-asset's
opus frame stream + tts lifecycle frames route through the same lock
so a concurrent admin call (e.g. a sound-turner head-turn) can't
interleave mid-frame. (Upstream's own chat-path writer doesn't take
the lock yet — this seam is where that lands.)
Reply correlation is deliberately NOT implemented: call_tool returns
the request id so a correlation layer can be added behind this
interface without touching the twelve callers again.
http_server.py also gains _dotty_resolve_conn(): one copy of the
named-or-first-device lookup instead of eight, which fixes the audit's
inject-text missing-`or {}` headers guard as a side effect. Handlers
shrink from ~50 lines to ~15.
Both compose files gain the new bind mount (volume/env parity kept).
Tests: tests/test_device_command.py (8 cases — monotonic/per-conn ids,
wire-shape parity with the old envelope, send serialization under
concurrency, http_server wiring, and a guard that the ms-truncated id
pattern never resurfaces in either file); the two existing http_server
harnesses now inject the real seam module. Full suite 320 passing,
coverage 69.1% (gate 56%).
Bench-pending: live-device smoke (LED pips, head-turn, take-photo,
play-asset) before release sign-off.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
What
Candidate 3 from the 2026-06-11 pre-release architecture review. Twelve call sites — the five
/xiaozhi/admin/*MCP handlers inhttp_server.pyand seven helpers inreceiveAudioHandle.py— each hand-rolled the same MCP JSON-RPC envelope, so every shared defect was twelve defects. All server→firmware tool calls now go through one module,custom-providers/xiaozhi-patches/device_command.py, mounted atcore/utils/device_command.py(same pattern astextUtils.py) so both patch surfaces import it.Audit bug classes killed
int(time.time()*1000) % 0x7FFFFFFFcollide within a millisecond (×12 sites)conn.websocket.send()concurrently with each other / play-asset's frame stream (websockets forbids interleaved sends)asyncio.Lock; every send routed through the seam is mutually exclusive — including play-asset's opus frames and tts lifecycle frames, so a concurrent head-turn can't interleave mid-frame_dotty_inject_textreadsconn.headerswithout theor {}guard used elsewhere_dotty_resolve_conn()/_dotty_conn_device_id()— one copy of the lookup instead of eightHonest scope notes:
call_toolreturns the request id so a correlation layer can be added behind this interface without touching the twelve callers again (the audit's "device-side failures invisible" finding remains open, now one-module-sized).Handlers shrink from ~50 lines to ~15; net −280 lines of copy-paste. Both compose files gain the bind mount (volume parity kept — the all-in-one's header rule).
Test plan
ruff check .clean; full suite 320 passing (107 root/pi_voice + 213 behaviour), combined coverage 69.1% (gate 56%); both compose files YAML-validatedtests/test_device_command.py(8 cases): monotonic + per-connection ids under a 1000-call burst, wire-shape parity with the old hand-rolled envelope, send serialization under concurrency (max-overlap == 1), http_server handler wiring (ids 1,2 on the wire), 503/fallback conn resolution, and the no-% 0x7FFFFFFF-anywhere guardtest_admin_auth_middleware.py,test_play_asset_route.py) now inject the real seam module instead of a MagicMockRefs the AUDIT-REPORT 2026-06-06 findings (ms-truncated ids / concurrent sends / inject-text headers nit). Independent of #158 and #159 except for a trivial CHANGELOG-adjacency conflict with whichever merges first (keep all entries).
🤖 Generated with Claude Code