refactor(examples)!: share JSON/text/MCP helpers in rtc-tcp-client demos - #14
Merged
Merged
Conversation
The same ~170-line block of JSON readers, base64 decode and session-token parsing sat in all five POSIX demos, so the same bugs sat in all five too, and the copies had begun to diverge (json_get_object vs json_get_object_raw, json_get_long present in some, different parse_token diagnostics). It moves into headers next to the existing demo_reconnect.h so a fix lands once: demo_json.h JSON readers, base64, parse_token(), bounded field copy demo_text.h tai_text_msg_t handling and stream reassembly demo_mcp.h device-side MCP request answering The shared version takes the superset of what the copies drifted into, so no demo loses behaviour. All five migrate onto it. The fixes below belong in the shared code rather than in five places: - tai_text_msg_t.text is a borrowed slice of the SDK receive buffer and is explicitly NOT NUL-terminated, but the demos ran strstr/strchr over it, reading past msg->len and eventually past the end of the tai_ctx_t allocation. All text handling is length-bounded or copies the bytes out. - devid / secret_key / local_key from argv were memcpy'd into iot_client_config_t's 32-byte fields with no length check, overflowing the stack-local config. demo_copy_field() rejects a value that does not fit. - The scanners are string- and escape-aware: a delimiter inside a JSON string no longer terminates a span (a track titled "Best Of ]" used to truncate the audios array), and \" \\ \/ \uXXXX with surrogate pairs decode, while NUL and lone surrogates are rejected. NLG prose is decoded before printing too, where \uXXXX-escaped Chinese used to reach the terminal as escapes. - A value that does not fit is rejected rather than truncated, since half a credential is worse than none — except for fields that are only printed, which truncate via json_get_display_string(): a song title past 255 bytes rendered as "(unknown)". parse_token() tells capacity apart from a wrong type and a malformed escape, and an out-of-range port is rejected instead of truncated modulo 65536. - demo_textbuf_accum() reassembles START/MIDDLE/END before a caller parses, which is what lets the music demo recognise a SKILL response split across chunks — parsed per chunk it never matched at all. The SDK drops empty text frames, so a stream ended by a bare zero-length END never completes; demo_textbuf_flush() delivers it at end-of-turn. - Two streams interleaving inside a turn cannot be demuxed (same event_id, constant data_id), so each loss is announced and counted in demo_textbuf_t.dropped; music_play_demo exits non-zero on it rather than reporting "no music skill response" and exiting 0 for a run that lost its payload. A stream displaced by a new START used to vanish without a word. A seq gap now warns and keeps accumulating, because the empty frames the SDK swallows consume a seq while carrying no bytes; DEMO_TEXT_SEQ_CHECK=2 drops instead, =0 skips the check. - Four demos answered every TAI_EVT_MCP_CMD with one canned reply that hardcoded "id":1 — JSON-RPC correlates on the id — and always used the tools/call result shape, so initialize and tools/list got the wrong body. Opting out is not available: the SDK's default session attributes declare deviceMcp.supportCustomMCP. demo_mcp_reply_no_tools() answers as a device with an empty catalog and stays silent for an id-less request, which is a notification (mcp_demo too, which used to answer "id":null). demo_mcp_copy_id() reads id and method from the top-level members only — a tools/call may carry an "id" of its own inside params.arguments — and refuses any id it cannot echo verbatim: an over-long quoted one (spliced in without its closing quote before) or an object/array one. Marked ! because two contracts change: json_get_string() fails instead of truncating (json_get_display_string() is the opt-in for printed values), and json_get_object_raw/json_get_array_raw are spelled json_get_object and json_get_array. All POSIX example targets build clean under -Wall -Wextra -Wshadow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
heshaoqiong-tuya
force-pushed
the
feature/rtc-tcp-client-demo-helpers
branch
from
August 11, 2026 08:08
6149396 to
ba4468f
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.
The same ~170-line block of JSON readers, base64 decode and session-token parsing sat in all five POSIX demos, so the same bugs sat in all five too, and the copies had begun to diverge (json_get_object vs json_get_object_raw, json_get_long present in some, different parse_token diagnostics). It moves into headers next to the existing demo_reconnect.h so a fix lands once:
demo_json.h JSON readers, base64, parse_token(), bounded field copy
demo_text.h tai_text_msg_t handling and stream reassembly
demo_mcp.h device-side MCP request answering
The shared version takes the superset of what the copies drifted into, so no demo loses behaviour. All five migrate onto it. The fixes below belong in the shared code rather than in five places:
tai_text_msg_t.text is a borrowed slice of the SDK receive buffer and is explicitly NOT NUL-terminated, but the demos ran strstr/strchr over it, reading past msg->len and eventually past the end of the tai_ctx_t allocation. All text handling is length-bounded or copies the bytes out.
devid / secret_key / local_key from argv were memcpy'd into iot_client_config_t's 32-byte fields with no length check, overflowing the stack-local config. demo_copy_field() rejects a value that does not fit.
The scanners are string- and escape-aware: a delimiter inside a JSON string no longer terminates a span (a track titled "Best Of ]" used to truncate the audios array), and " \ / \uXXXX with surrogate pairs decode, while NUL and lone surrogates are rejected. NLG prose is decoded before printing too, where \uXXXX-escaped Chinese used to reach the terminal as escapes.
A value that does not fit is rejected rather than truncated, since half a credential is worse than none — except for fields that are only printed, which truncate via json_get_display_string(): a song title past 255 bytes rendered as "(unknown)". parse_token() tells capacity apart from a wrong type and a malformed escape, and an out-of-range port is rejected instead of truncated modulo 65536.
demo_textbuf_accum() reassembles START/MIDDLE/END before a caller parses, which is what lets the music demo recognise a SKILL response split across chunks — parsed per chunk it never matched at all. The SDK drops empty text frames, so a stream ended by a bare zero-length END never completes; demo_textbuf_flush() delivers it at end-of-turn.
Two streams interleaving inside a turn cannot be demuxed (same event_id, constant data_id), so each loss is announced and counted in demo_textbuf_t.dropped; music_play_demo exits non-zero on it rather than reporting "no music skill response" and exiting 0 for a run that lost its payload. A stream displaced by a new START used to vanish without a word. A seq gap now warns and keeps accumulating, because the empty frames the SDK swallows consume a seq while carrying no bytes; DEMO_TEXT_SEQ_CHECK=2 drops instead, =0 skips the check.
Four demos answered every TAI_EVT_MCP_CMD with one canned reply that hardcoded "id":1 — JSON-RPC correlates on the id — and always used the tools/call result shape, so initialize and tools/list got the wrong body. Opting out is not available: the SDK's default session attributes declare deviceMcp.supportCustomMCP. demo_mcp_reply_no_tools() answers as a device with an empty catalog and stays silent for an id-less request, which is a notification (mcp_demo too, which used to answer "id":null). demo_mcp_copy_id() reads id and method from the top-level members only — a tools/call may carry an "id" of its own inside params.arguments — and refuses any id it cannot echo verbatim: an over-long quoted one (spliced in without its closing quote before) or an object/array one.
Four demo headers and the device-MCP guide passed -DAGENTIC_KIT_BUILD_EXAMPLES=ON to a standalone examples/posix configure, which forces the option OFF to stop recursing and so discards it. The flag belongs to the root-project form. No CMake behaviour changed.
Marked ! because two contracts change: json_get_string() fails instead of truncating (json_get_display_string() is the opt-in for printed values), and json_get_object_raw/json_get_array_raw are spelled json_get_object and json_get_array.
All POSIX example targets build clean under -Wall -Wextra -Wshadow.