From d57577637c1c1a16176d63d5a89e9eec3c1e4d7c Mon Sep 17 00:00:00 2001 From: tanaka takashi Date: Fri, 17 Jul 2026 12:15:41 +0900 Subject: fix: sessions paginationとhealth診断を改善 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - sessions cursorのエンコード不具合と長いsession IDのバッファ問題を修正 - response budgetに基づくsessionsページングを追加 - get_system_healthにsession_countを追加 - metadata_jsonのフィールド別エラー診断を追加 - CRUD互換性とDebug/Release/ASanテストを確認 --- docs/mcp.md | 7 ++ src/mcp/tools/common/common.h | 13 ++- src/mcp/tools/get_system_health.c | 7 ++ src/mcp/tools/read_messages.c | 185 ++++++++++++++++++++++++-------------- src/mcp/tools/update_message.c | 22 ++++- src/mcp/tools/write_message.c | 169 ++++++++++++++++++++++++---------- tests/mcp_protocol_test.py | 168 +++++++++++++++++++++++++++++++++- 7 files changed, 450 insertions(+), 121 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 639946f..788fc84 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -28,6 +28,9 @@ application errorではMCP tool resultの`isError`がtrueになり、`text`は } ``` +`metadata_json`の検証errorでは、`error.message`に問題のあるfield名と制約が含まれる。 +入力値そのものはmessageへ転載されない。allocation失敗は`INTERNAL_ERROR`として区別される。 + 主なerror code: | Code | 意味 | @@ -217,6 +220,8 @@ session内のlive messageを作成順に返す。 ### `sessions` view session一覧を時刻とIDの安定順序でpage取得する。`session_id`は使用しない。 +`limit`到達時に加え、response budgetへ到達した場合もそこでpageを終了し、続きがあれば +`nextCursor`を返す。`representative_summary`の内容は短縮されない。 ```json { @@ -359,6 +364,7 @@ parameterなし。永続化状態、logical I/O counter、memory estimateを返 "log_size": 524288, "snapshot_size": 262144, "active_messages": 120, + "session_count": 18, "deleted_messages": 3, "last_sequence": 150, "update_records": 12, @@ -386,5 +392,6 @@ parameterなし。永続化状態、logical I/O counter、memory estimateを返 ``` `status`は`last_error`が空なら`OK`、診断messageが残っていれば`DEGRADED`となる。 +`session_count`は現在live messageを持つsessionの数で、memory上のhistory indexから集計される。 `memory_estimate`はprojectが管理する主要allocationの概算で、runtimeやallocator overheadは 含まない。 diff --git a/src/mcp/tools/common/common.h b/src/mcp/tools/common/common.h index 741b2ce..0dedca8 100644 --- a/src/mcp/tools/common/common.h +++ b/src/mcp/tools/common/common.h @@ -10,8 +10,17 @@ void tiny_mcp_tool_add_error(mcpc_ucbr_t *result, const char *code, const char * void tiny_mcp_tool_add_json(mcpc_ucbr_t *result, tiny_json_buffer_t *json); char *tiny_mcp_tool_get_string(const mcpc_tool_t *tool, const char *name, size_t maximum); void tiny_mcp_tool_append_message(tiny_json_buffer_t *json, const tiny_message_t *message); -bool -tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata_mask_t *present); +typedef enum tiny_metadata_parse_status +{ + TINY_METADATA_PARSE_OK = 0, + TINY_METADATA_PARSE_INVALID, + TINY_METADATA_PARSE_NO_MEMORY +} tiny_metadata_parse_status_t; + +tiny_metadata_parse_status_t tiny_mcp_parse_metadata(const char *json, + tiny_message_t *message, + tiny_metadata_mask_t *present, + const char **error); mcpc_tool_t *tiny_mcp_tool_new(const char *name, const char *description, mcpc_tcallcb_t callback); bool tiny_mcp_tool_add_property(mcpc_tool_t *tool, diff --git a/src/mcp/tools/get_system_health.c b/src/mcp/tools/get_system_health.c index 893a3b1..4189db3 100644 --- a/src/mcp/tools/get_system_health.c +++ b/src/mcp/tools/get_system_health.c @@ -15,6 +15,11 @@ call_get_system_health(const mcpc_tool_t *tool, mcpc_ucbr_t *result) tiny_mcp_tool_add_error(result, "INTERNAL_ERROR", "tool context is unavailable"); return; } + if (!tiny_mcp_tools_ensure_history_index(context) || !context->history_index) + { + tiny_mcp_tool_add_error(result, "INTERNAL_ERROR", "history index allocation failed"); + return; + } tiny_store_health(context->store, &health); tiny_json_init(&json); tiny_json_raw(&json, "{\"status\":"); @@ -27,6 +32,8 @@ call_get_system_health(const mcpc_tool_t *tool, mcpc_ucbr_t *result) tiny_json_u64(&json, health.snapshot_size); tiny_json_raw(&json, ",\"active_messages\":"); tiny_json_u64(&json, health.active_messages); + tiny_json_raw(&json, ",\"session_count\":"); + tiny_json_u64(&json, tiny_history_index_session_count(context->history_index)); tiny_json_raw(&json, ",\"deleted_messages\":"); tiny_json_u64(&json, health.deleted_messages); tiny_json_raw(&json, ",\"last_sequence\":"); diff --git a/src/mcp/tools/read_messages.c b/src/mcp/tools/read_messages.c index 109becb..131c513 100644 --- a/src/mcp/tools/read_messages.c +++ b/src/mcp/tools/read_messages.c @@ -15,8 +15,10 @@ #define READ_DEFAULT_LIMIT 100 #define READ_MAX_LIMIT 1000 #define READ_RESPONSE_BUDGET (8U * 1024U * 1024U - 64U * 1024U) -#define CURSOR_BYTES 47U -#define CURSOR_TEXT_MAX 64U +#define HISTORY_CURSOR_BYTES 47U +#define HISTORY_CURSOR_TEXT_MAX 64U +#define SESSION_CURSOR_BYTES_MAX (1U + 8U + 4U + TINY_SESSION_ID_MAX + 4U) +#define SESSION_CURSOR_TEXT_MAX (((SESSION_CURSOR_BYTES_MAX * 4U + 2U) / 3U) + 1U) typedef struct history_cursor { @@ -101,16 +103,16 @@ base64_value(unsigned char value) } static bool -base64url_decode(const char *input, unsigned char output[CURSOR_BYTES]) +base64url_decode_n(const char *input, unsigned char *output, size_t capacity, size_t *length) { - size_t length = strlen(input), source = 0, target = 0; + size_t input_length = strlen(input), source = 0, target = 0; uint32_t accumulator = 0; unsigned bits = 0; - if (length != 63) + if (!output || !length || input_length == 0 || input_length % 4 == 1) { return false; } - while (source < length) + while (source < input_length) { int value = base64_value((unsigned char)input[source++]); if (value < 0) @@ -122,7 +124,7 @@ base64url_decode(const char *input, unsigned char output[CURSOR_BYTES]) if (bits >= 8) { bits -= 8; - if (target >= CURSOR_BYTES) + if (target >= capacity) { return false; } @@ -130,26 +132,31 @@ base64url_decode(const char *input, unsigned char output[CURSOR_BYTES]) accumulator &= bits ? (UINT32_C(1) << bits) - 1 : 0; } } - return target == CURSOR_BYTES && accumulator == 0; + *length = target; + return accumulator == 0; } static bool -cursor_encode(const char *session, const tiny_message_t *message, char output[CURSOR_TEXT_MAX]) +cursor_encode(const char *session, + const tiny_message_t *message, + char output[HISTORY_CURSOR_TEXT_MAX]) { - unsigned char data[CURSOR_BYTES] = { 0 }; + unsigned char data[HISTORY_CURSOR_BYTES] = { 0 }; data[0] = 1; tiny_codec_put_u64(data + 1, session_hash(session)); tiny_codec_put_u64(data + 9, (uint64_t)message->timestamp_ms); memcpy(data + 17, message->message_id, 26); tiny_codec_put_u32(data + 43, tiny_codec_crc32(data, 43)); - return base64url_encode(data, sizeof(data), output, CURSOR_TEXT_MAX) == 63; + return base64url_encode(data, sizeof(data), output, HISTORY_CURSOR_TEXT_MAX) == 63; } static bool cursor_decode(const char *session, const char *value, history_cursor_t *cursor) { - unsigned char data[CURSOR_BYTES]; - if (!value || !value[0] || !base64url_decode(value, data) || data[0] != 1 + unsigned char data[HISTORY_CURSOR_BYTES]; + size_t length = 0; + if (!value || strlen(value) != 63 || !base64url_decode_n(value, data, sizeof(data), &length) + || length != HISTORY_CURSOR_BYTES || data[0] != 1 || tiny_codec_get_u64(data + 1) != session_hash(session) || tiny_codec_get_u32(data + 43) != tiny_codec_crc32(data, 43)) { @@ -538,16 +545,24 @@ simple_cursor(const char *cursor, bool *valid) } static bool -session_cursor_encode(int64_t last_message_at, const char *session_id, char output[CURSOR_TEXT_MAX]) +session_cursor_encode(int64_t last_message_at, + const char *session_id, + char output[SESSION_CURSOR_TEXT_MAX]) { - unsigned char data[CURSOR_BYTES] = { 0 }; + unsigned char data[SESSION_CURSOR_BYTES_MAX] = { 0 }; + size_t session_length = strlen(session_id); + size_t data_length; + if (session_length == 0 || session_length > TINY_SESSION_ID_MAX) + { + return false; + } + data_length = 1U + 8U + 4U + session_length + 4U; data[0] = 2; tiny_codec_put_u64(data + 1, (uint64_t)last_message_at); - tiny_codec_put_u32(data + 9, - tiny_codec_crc32((const unsigned char *)session_id, strlen(session_id))); - memcpy(data + 13, session_id, strlen(session_id)); - tiny_codec_put_u32(data + 43, tiny_codec_crc32(data, 43)); - return base64url_encode(data, sizeof(data), output, CURSOR_TEXT_MAX) == 63; + tiny_codec_put_u32(data + 9, (uint32_t)session_length); + memcpy(data + 13, session_id, session_length); + tiny_codec_put_u32(data + data_length - 4U, tiny_codec_crc32(data, data_length - 4U)); + return base64url_encode(data, data_length, output, SESSION_CURSOR_TEXT_MAX) > 0; } static bool @@ -555,22 +570,25 @@ session_cursor_decode(const char *cursor, int64_t *last_message_at, char session_id[TINY_SESSION_ID_MAX + 1]) { - unsigned char data[CURSOR_BYTES]; - size_t length; - if (!cursor || !cursor[0] || !base64url_decode(cursor, data) || data[0] != 2) + unsigned char data[SESSION_CURSOR_BYTES_MAX]; + size_t decoded_length = 0, session_length; + if (!cursor || !cursor[0] || !base64url_decode_n(cursor, data, sizeof(data), &decoded_length) + || decoded_length < 18U || data[0] != 2) { return false; } *last_message_at = (int64_t)tiny_codec_get_u64(data + 1); - length = tiny_codec_get_u32(data + 9); - if (length == 0 || length > TINY_SESSION_ID_MAX - || tiny_codec_crc32(data, 43) != tiny_codec_get_u32(data + 43)) + session_length = tiny_codec_get_u32(data + 9); + if (*last_message_at < 0 || session_length == 0 || session_length > TINY_SESSION_ID_MAX + || decoded_length != 1U + 8U + 4U + session_length + 4U + || tiny_codec_crc32(data, decoded_length - 4U) + != tiny_codec_get_u32(data + decoded_length - 4U)) { return false; } - memcpy(session_id, data + 13, length); - session_id[length] = '\0'; - return tiny_utf8_valid(session_id, length); + memcpy(session_id, data + 13, session_length); + session_id[session_length] = '\0'; + return tiny_utf8_valid(session_id, session_length); } static int @@ -588,14 +606,15 @@ compare_session_by_last_then_id(const void *left, const void *right) static void call_sessions_view(tiny_mcp_tools_context_t *context, const mcpc_tool_t *tool, mcpc_ucbr_t *result) { - char *cursor = tiny_mcp_tool_get_string(tool, "cursor", CURSOR_TEXT_MAX); + char *cursor = tiny_mcp_tool_get_string(tool, "cursor", SESSION_CURSOR_TEXT_MAX - 1U); int32_t limit = READ_DEFAULT_LIMIT; tiny_json_buffer_t json; mcpc_errcode_t limit_status = mcpc_tool_get_tpropval_i32(tool, (const char8_t *)"limit", &limit); int64_t cursor_last = 0; char cursor_session[TINY_SESSION_ID_MAX + 1] = { 0 }; bool has_cursor = cursor && cursor[0]; - size_t emitted = 0, index, position; + size_t emitted = 0, index, position, last_emitted_position = SIZE_MAX, outer_size = 256; + bool more = false; const tiny_history_index_t *history; tiny_history_index_session_info_t *sessions = NULL; size_t session_count = 0; @@ -680,49 +699,85 @@ call_sessions_view(tiny_mcp_tools_context_t *context, const mcpc_tool_t *tool, m { continue; } - if (emitted++) - { - tiny_json_raw(&json, ","); - } summary = representative->message.metadata.summary ? representative->message.metadata.summary : representative->message.content; - tiny_json_raw(&json, "{\"session_id\":"); - tiny_json_string(&json, session_id); - tiny_json_raw(&json, ",\"message_count\":"); - tiny_json_u64(&json, count); - tiny_json_raw(&json, ",\"kind_counts\":{\"conversation\":"); - tiny_json_i64(&json, kind_counts[0]); - tiny_json_raw(&json, ",\"episode\":"); - tiny_json_i64(&json, kind_counts[1]); - tiny_json_raw(&json, ",\"semantic\":"); - tiny_json_i64(&json, kind_counts[2]); - tiny_json_raw(&json, ",\"procedure\":"); - tiny_json_i64(&json, kind_counts[3]); - tiny_json_raw(&json, ",\"preference\":"); - tiny_json_i64(&json, kind_counts[4]); - tiny_json_raw(&json, ",\"summary\":"); - tiny_json_i64(&json, kind_counts[5]); - tiny_json_raw(&json, "},\"representative_summary\":"); - tiny_json_string(&json, summary); - tiny_json_raw(&json, ",\"started_at\":"); - tiny_json_i64(&json, started); - tiny_json_raw(&json, ",\"last_message_at\":"); - tiny_json_i64(&json, ended); - tiny_json_raw(&json, "}"); + { + tiny_json_buffer_t item; + size_t item_size; + tiny_json_init(&item); + tiny_json_raw(&item, "{\"session_id\":"); + tiny_json_string(&item, session_id); + tiny_json_raw(&item, ",\"message_count\":"); + tiny_json_u64(&item, count); + tiny_json_raw(&item, ",\"kind_counts\":{\"conversation\":"); + tiny_json_i64(&item, kind_counts[0]); + tiny_json_raw(&item, ",\"episode\":"); + tiny_json_i64(&item, kind_counts[1]); + tiny_json_raw(&item, ",\"semantic\":"); + tiny_json_i64(&item, kind_counts[2]); + tiny_json_raw(&item, ",\"procedure\":"); + tiny_json_i64(&item, kind_counts[3]); + tiny_json_raw(&item, ",\"preference\":"); + tiny_json_i64(&item, kind_counts[4]); + tiny_json_raw(&item, ",\"summary\":"); + tiny_json_i64(&item, kind_counts[5]); + tiny_json_raw(&item, "},\"representative_summary\":"); + tiny_json_string(&item, summary); + tiny_json_raw(&item, ",\"started_at\":"); + tiny_json_i64(&item, started); + tiny_json_raw(&item, ",\"last_message_at\":"); + tiny_json_i64(&item, ended); + tiny_json_raw(&item, "}"); + item_size = item.failed ? 0U : escaped_size(item.data, item.length) + (emitted ? 2U : 0U); + if (item.failed || (emitted && outer_size + item_size > READ_RESPONSE_BUDGET)) + { + if (item.failed) + { + json.failed = true; + } + else + { + more = true; + } + tiny_json_free(&item); + break; + } + if (emitted) + { + tiny_json_raw(&json, ","); + } + tiny_json_raw_n(&json, item.data, item.length); + tiny_json_free(&item); + outer_size += item_size; + emitted++; + last_emitted_position = position; + } } tiny_json_raw(&json, "]"); - if (emitted == (size_t)limit && position < session_count) + if (!more && emitted == (size_t)limit && position < session_count) { - char next[CURSOR_TEXT_MAX]; - if (session_cursor_encode( - sessions[position - 1].last_message_at, sessions[position - 1].session_id, next)) + more = true; + } + if (more && last_emitted_position != SIZE_MAX) + { + char next[SESSION_CURSOR_TEXT_MAX]; + if (session_cursor_encode(sessions[last_emitted_position].last_message_at, + sessions[last_emitted_position].session_id, + next)) { tiny_json_raw(&json, ",\"nextCursor\":"); tiny_json_string(&json, next); } } tiny_json_raw(&json, "}"); - tiny_mcp_tool_add_json(result, &json); + if (json.failed) + { + tiny_mcp_tool_add_error(result, "INTERNAL_ERROR", "sessions response allocation failed"); + } + else + { + tiny_mcp_tool_add_json(result, &json); + } tiny_json_free(&json); free(sessions); free(cursor); @@ -734,7 +789,7 @@ call_read_messages(const mcpc_tool_t *tool, mcpc_ucbr_t *result) tiny_mcp_tools_context_t *context = tiny_mcp_tools_context_current(); char *view = tiny_mcp_tool_get_string(tool, "view", 16); char *session = tiny_mcp_tool_get_string(tool, "session_id", TINY_SESSION_ID_MAX); - char *cursor_text = tiny_mcp_tool_get_string(tool, "cursor", CURSOR_TEXT_MAX); + char *cursor_text = tiny_mcp_tool_get_string(tool, "cursor", SESSION_CURSOR_TEXT_MAX - 1U); int32_t requested_limit = READ_DEFAULT_LIMIT; mcpc_errcode_t limit_status = mcpc_tool_get_tpropval_i32(tool, (const char8_t *)"limit", &requested_limit); @@ -860,7 +915,7 @@ call_read_messages(const mcpc_tool_t *tool, mcpc_ucbr_t *result) tiny_json_raw(&json, "]"); if (more && last) { - char next_cursor[CURSOR_TEXT_MAX]; + char next_cursor[HISTORY_CURSOR_TEXT_MAX]; if (!cursor_encode(session, last, next_cursor)) { json.failed = true; diff --git a/src/mcp/tools/update_message.c b/src/mcp/tools/update_message.c index aca1bed..9b02a6f 100644 --- a/src/mcp/tools/update_message.c +++ b/src/mcp/tools/update_message.c @@ -22,6 +22,8 @@ call_update_message(const mcpc_tool_t *tool, mcpc_ucbr_t *result) tiny_json_buffer_t json; bool changed = false; tiny_metadata_mask_t metadata_mask = 0; + const char *metadata_error = NULL; + tiny_metadata_parse_status_t metadata_status = TINY_METADATA_PARSE_OK; tiny_message_init(&parsed); tiny_message_init(&updated); patch.content = content; @@ -29,10 +31,22 @@ call_update_message(const mcpc_tool_t *tool, mcpc_ucbr_t *result) patch.metadata_mask = 0; patch.expected_revision = (uint64_t)expected; patch.has_expected_revision = expected_status == MCPC_EC_0; - if (!context || !message_id || (!content && !metadata_json) - || (metadata_json && !tiny_mcp_parse_metadata(metadata_json, &parsed, &metadata_mask)) - || (expected_status != MCPC_EC_0 && expected_status != MCPC_EC_ANYDATA_NOTFOUND) - || (patch.has_expected_revision && expected < 1)) + if (metadata_json) + { + metadata_status + = tiny_mcp_parse_metadata(metadata_json, &parsed, &metadata_mask, &metadata_error); + } + if (metadata_status == TINY_METADATA_PARSE_NO_MEMORY) + { + tiny_mcp_tool_add_error(result, "INTERNAL_ERROR", metadata_error); + } + else if (metadata_status == TINY_METADATA_PARSE_INVALID) + { + tiny_mcp_tool_add_error(result, "VALIDATION_ERROR", metadata_error); + } + else if (!context || !message_id || (!content && !metadata_json) + || (expected_status != MCPC_EC_0 && expected_status != MCPC_EC_ANYDATA_NOTFOUND) + || (patch.has_expected_revision && expected < 1)) { tiny_mcp_tool_add_error(result, "VALIDATION_ERROR", "invalid update_message arguments"); } diff --git a/src/mcp/tools/write_message.c b/src/mcp/tools/write_message.c index 3d7cb29..51fd05b 100644 --- a/src/mcp/tools/write_message.c +++ b/src/mcp/tools/write_message.c @@ -9,8 +9,8 @@ #include "base/allocator.h" -static bool -metadata_has_only_known_fields(const char *json, int length) +static tiny_metadata_parse_status_t +metadata_check_known_fields(const char *json, int length, const char **error) { int offset = 0, key_offset, key_length, value_offset, value_length, value_type; while ((offset = mjson_next(json, @@ -30,7 +30,8 @@ metadata_has_only_known_fields(const char *json, int length) (void)value_type; if (decoded <= 0 || decoded >= (int)sizeof(key)) { - return false; + *error = "metadata_json contains an invalid field name"; + return TINY_METADATA_PARSE_INVALID; } key[decoded] = '\0'; if (strcmp(key, "tool_call_id") != 0 && strcmp(key, "tags") != 0 @@ -38,14 +39,18 @@ metadata_has_only_known_fields(const char *json, int length) && strcmp(key, "importance") != 0 && strcmp(key, "summary") != 0 && strcmp(key, "relations") != 0 && strcmp(key, "source_message_ids") != 0) { - return false; + *error = "metadata_json contains an unknown field"; + return TINY_METADATA_PARSE_INVALID; } } - return true; + return TINY_METADATA_PARSE_OK; } -bool -tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata_mask_t *present) +tiny_metadata_parse_status_t +tiny_mcp_parse_metadata(const char *json, + tiny_message_t *message, + tiny_metadata_mask_t *present, + const char **error) { const char *tool_call = NULL, *tier = NULL, *tags = NULL, *kind = NULL, *summary = NULL; const char *relations = NULL, *sources = NULL; @@ -58,14 +63,26 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata int relations_type, sources_type; double importance = 50; tiny_metadata_mask_t fields = 0; + if (error) + { + *error = NULL; + } + if (!json || !message || !error) + { + return TINY_METADATA_PARSE_INVALID; + } if (strcmp(json, "{}") == 0) { - return true; + return TINY_METADATA_PARSE_OK; } - if (mjson_find(json, length, "$", NULL, NULL) != MJSON_TOK_OBJECT - || !metadata_has_only_known_fields(json, length)) + if (mjson_find(json, length, "$", NULL, NULL) != MJSON_TOK_OBJECT) { - return false; + *error = "metadata_json must be a JSON object"; + return TINY_METADATA_PARSE_INVALID; + } + if (metadata_check_known_fields(json, length, error) != TINY_METADATA_PARSE_OK) + { + return TINY_METADATA_PARSE_INVALID; } tool_call_type = mjson_find(json, length, "$.tool_call_id", &tool_call, &tool_call_len); tier_type = mjson_find(json, length, "$.memory_tier", &tier, &tier_len); @@ -75,17 +92,25 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata importance_type = mjson_find(json, length, "$.importance", NULL, NULL); relations_type = mjson_find(json, length, "$.relations", &relations, &relations_len); sources_type = mjson_find(json, length, "$.source_message_ids", &sources, &sources_len); - if ((tool_call_type != MJSON_TOK_INVALID && tool_call_type != MJSON_TOK_STRING) - || (tier_type != MJSON_TOK_INVALID && tier_type != MJSON_TOK_STRING) - || (tags_type != MJSON_TOK_INVALID && tags_type != MJSON_TOK_ARRAY) - || (kind_type != MJSON_TOK_INVALID && kind_type != MJSON_TOK_STRING) - || (summary_type != MJSON_TOK_INVALID && summary_type != MJSON_TOK_STRING) - || (importance_type != MJSON_TOK_INVALID && importance_type != MJSON_TOK_NUMBER) - || (relations_type != MJSON_TOK_INVALID && relations_type != MJSON_TOK_ARRAY) - || (sources_type != MJSON_TOK_INVALID && sources_type != MJSON_TOK_ARRAY)) - { - return false; - } +#define REQUIRE_METADATA_TYPE(actual, expected, field, type_name) \ + do \ + { \ + if ((actual) != MJSON_TOK_INVALID && (actual) != (expected)) \ + { \ + *error = "metadata_json." field " must be " type_name; \ + return TINY_METADATA_PARSE_INVALID; \ + } \ + } \ + while (0) + REQUIRE_METADATA_TYPE(tool_call_type, MJSON_TOK_STRING, "tool_call_id", "a string"); + REQUIRE_METADATA_TYPE(tier_type, MJSON_TOK_STRING, "memory_tier", "a string"); + REQUIRE_METADATA_TYPE(tags_type, MJSON_TOK_ARRAY, "tags", "an array"); + REQUIRE_METADATA_TYPE(kind_type, MJSON_TOK_STRING, "memory_kind", "a string"); + REQUIRE_METADATA_TYPE(summary_type, MJSON_TOK_STRING, "summary", "a string"); + REQUIRE_METADATA_TYPE(importance_type, MJSON_TOK_NUMBER, "importance", "an integer"); + REQUIRE_METADATA_TYPE(relations_type, MJSON_TOK_ARRAY, "relations", "an array"); + REQUIRE_METADATA_TYPE(sources_type, MJSON_TOK_ARRAY, "source_message_ids", "an array"); +#undef REQUIRE_METADATA_TYPE fields |= tool_call_type != MJSON_TOK_INVALID ? TINY_METADATA_TOOL_CALL_ID : 0; fields |= tags_type != MJSON_TOK_INVALID ? TINY_METADATA_TAGS : 0; fields |= tier_type != MJSON_TOK_INVALID ? TINY_METADATA_TIER : 0; @@ -114,7 +139,8 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata if (value_type != MJSON_TOK_OBJECT || message->metadata.relation_count >= TINY_RELATION_COUNT_MAX) { - return false; + *error = "metadata_json.relations must contain at most 32 objects"; + return TINY_METADATA_PARSE_INVALID; } decoded_type = mjson_get_string( relations + value_offset, value_length, "$.type", type, sizeof(type)); @@ -126,20 +152,23 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata if (decoded_type <= 0 || decoded_type >= (int)sizeof(type) || decoded_id != TINY_MESSAGE_ID_LENGTH) { - return false; + *error = "metadata_json.relations entries require type and a 26-character message_id"; + return TINY_METADATA_PARSE_INVALID; } type[decoded_type] = '\0'; relation.message_id[decoded_id] = '\0'; if (!tiny_relation_type_parse(type, &relation.type) || !tiny_message_id_valid(relation.message_id)) { - return false; + *error = "metadata_json.relations contains an invalid type or message_id"; + return TINY_METADATA_PARSE_INVALID; } resized = realloc(message->metadata.relations, (message->metadata.relation_count + 1) * sizeof(*resized)); if (!resized) { - return false; + *error = "metadata_json allocation failed"; + return TINY_METADATA_PARSE_NO_MEMORY; } message->metadata.relations = resized; message->metadata.relations[message->metadata.relation_count++] = relation; @@ -164,43 +193,53 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata if (value_type != MJSON_TOK_STRING || message->metadata.source_count >= TINY_SOURCE_COUNT_MAX) { - return false; + *error = "metadata_json.source_message_ids must contain at most 64 strings"; + return TINY_METADATA_PARSE_INVALID; } id = malloc(TINY_MESSAGE_ID_LENGTH + 1); if (!id) { - return false; + *error = "metadata_json allocation failed"; + return TINY_METADATA_PARSE_NO_MEMORY; } decoded = mjson_get_string( sources + value_offset, value_length, "$", id, TINY_MESSAGE_ID_LENGTH + 1); if (decoded != TINY_MESSAGE_ID_LENGTH) { free(id); - return false; + *error = "metadata_json.source_message_ids entries must be 26-character ULIDs"; + return TINY_METADATA_PARSE_INVALID; } id[decoded] = '\0'; if (!tiny_message_id_valid(id)) { free(id); - return false; + *error = "metadata_json.source_message_ids contains an invalid ULID"; + return TINY_METADATA_PARSE_INVALID; } resized = realloc(message->metadata.source_message_ids, (message->metadata.source_count + 1) * sizeof(*resized)); if (!resized) { free(id); - return false; + *error = "metadata_json allocation failed"; + return TINY_METADATA_PARSE_NO_MEMORY; } message->metadata.source_message_ids = resized; message->metadata.source_message_ids[message->metadata.source_count++] = id; } } - if (importance_type == MJSON_TOK_NUMBER - && mjson_get_number(json, length, "$.importance", &importance) > 0) + if (importance_type == MJSON_TOK_NUMBER) { + if (mjson_get_number(json, length, "$.importance", &importance) <= 0) + { + *error = "metadata_json.importance must be an integer from 0 to 100"; + return TINY_METADATA_PARSE_INVALID; + } if (importance < 0 || importance > 100 || importance != (double)(uint8_t)importance) { - return false; + *error = "metadata_json.importance must be an integer from 0 to 100"; + return TINY_METADATA_PARSE_INVALID; } message->metadata.importance = (uint8_t)importance; } @@ -210,12 +249,14 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata int decoded = mjson_get_string(json, length, "$.memory_kind", value, sizeof(value)); if (decoded <= 0 || decoded >= (int)sizeof(value)) { - return false; + *error = "metadata_json.memory_kind contains an invalid value"; + return TINY_METADATA_PARSE_INVALID; } value[decoded] = '\0'; if (!tiny_kind_parse(value, &message->metadata.memory_kind)) { - return false; + *error = "metadata_json.memory_kind contains an invalid value"; + return TINY_METADATA_PARSE_INVALID; } } if (summary_type == MJSON_TOK_STRING) @@ -224,14 +265,16 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata message->metadata.summary = malloc(TINY_SUMMARY_MAX + 1); if (!message->metadata.summary) { - return false; + *error = "metadata_json allocation failed"; + return TINY_METADATA_PARSE_NO_MEMORY; } decoded = mjson_get_string( json, length, "$.summary", message->metadata.summary, TINY_SUMMARY_MAX + 1); if (decoded < 0 || decoded > TINY_SUMMARY_MAX || (decoded && !tiny_utf8_valid(message->metadata.summary, (size_t)decoded))) { - return false; + *error = "metadata_json.summary must be valid UTF-8 up to 4096 bytes"; + return TINY_METADATA_PARSE_INVALID; } message->metadata.summary[decoded] = '\0'; if (decoded == 0) @@ -246,7 +289,8 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata message->metadata.tool_call_id = malloc(TINY_TOOL_CALL_ID_MAX + 1); if (!message->metadata.tool_call_id) { - return false; + *error = "metadata_json allocation failed"; + return TINY_METADATA_PARSE_NO_MEMORY; } decoded = mjson_get_string(json, length, @@ -256,7 +300,8 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata if (decoded < 0 || decoded > TINY_TOOL_CALL_ID_MAX || !tiny_utf8_valid(message->metadata.tool_call_id, (size_t)decoded)) { - return false; + *error = "metadata_json.tool_call_id must be valid UTF-8 up to 256 bytes"; + return TINY_METADATA_PARSE_INVALID; } message->metadata.tool_call_id[decoded] = '\0'; } @@ -266,16 +311,19 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata int decoded = mjson_get_string(json, length, "$.memory_tier", tier_value, sizeof(tier_value)); if (decoded < 0 || decoded >= (int)sizeof(tier_value)) { - return false; + *error = "metadata_json.memory_tier contains an invalid value"; + return TINY_METADATA_PARSE_INVALID; } tier_value[decoded] = '\0'; if (!tiny_tier_parse(tier_value, &message->metadata.memory_tier)) { - return false; + *error = "metadata_json.memory_tier contains an invalid value"; + return TINY_METADATA_PARSE_INVALID; } } if (tags_type == MJSON_TOK_ARRAY) { + offset = 0; while ((offset = mjson_next(tags, tags_len, offset, @@ -291,18 +339,21 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata int decoded; if (value_type != MJSON_TOK_STRING || message->metadata.tag_count >= TINY_TAG_COUNT_MAX) { - return false; + *error = "metadata_json.tags must contain at most 32 strings"; + return TINY_METADATA_PARSE_INVALID; } tag = malloc(TINY_TAG_MAX + 1); if (!tag) { - return false; + *error = "metadata_json allocation failed"; + return TINY_METADATA_PARSE_NO_MEMORY; } decoded = mjson_get_string(tags + value_offset, value_length, "$", tag, TINY_TAG_MAX + 1); if (decoded <= 0 || decoded > TINY_TAG_MAX || !tiny_utf8_valid(tag, (size_t)decoded)) { free(tag); - return false; + *error = "metadata_json.tags entries must be non-empty valid UTF-8 up to 128 bytes"; + return TINY_METADATA_PARSE_INVALID; } tag[decoded] = '\0'; resized = realloc(message->metadata.tags, @@ -310,7 +361,8 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata if (!resized) { free(tag); - return false; + *error = "metadata_json allocation failed"; + return TINY_METADATA_PARSE_NO_MEMORY; } message->metadata.tags = resized; message->metadata.tags[message->metadata.tag_count++] = tag; @@ -320,7 +372,12 @@ tiny_mcp_parse_metadata(const char *json, tiny_message_t *message, tiny_metadata { *present = fields; } - return tiny_metadata_valid(&message->metadata); + if (!tiny_metadata_valid(&message->metadata)) + { + *error = "metadata_json contains duplicate or inconsistent metadata values"; + return TINY_METADATA_PARSE_INVALID; + } + return TINY_METADATA_PARSE_OK; } static void @@ -330,6 +387,8 @@ call_write_message(const mcpc_tool_t *tool, mcpc_ucbr_t *result) tiny_message_t input, written; tiny_store_status_t status; char *role = NULL, *metadata = NULL; + const char *metadata_error = NULL; + tiny_metadata_parse_status_t metadata_status = TINY_METADATA_PARSE_OK; tiny_json_buffer_t json; tiny_message_init(&input); tiny_message_init(&written); @@ -338,12 +397,24 @@ call_write_message(const mcpc_tool_t *tool, mcpc_ucbr_t *result) input.content = tiny_mcp_tool_get_string(tool, "content", TINY_CONTENT_MAX); metadata = tiny_mcp_tool_get_string(tool, "metadata_json", TINY_CONTENT_MAX); bool valid = context && input.session_id && input.session_id[0] && role && input.content - && input.content[0] && tiny_role_parse(role, &input.role) - && tiny_mcp_parse_metadata(metadata ? metadata : "{}", &input, NULL); + && input.content[0] && tiny_role_parse(role, &input.role); + if (valid) + { + metadata_status + = tiny_mcp_parse_metadata(metadata ? metadata : "{}", &input, NULL, &metadata_error); + } if (!valid) { tiny_mcp_tool_add_error(result, "VALIDATION_ERROR", "invalid write_message arguments"); } + else if (metadata_status == TINY_METADATA_PARSE_NO_MEMORY) + { + tiny_mcp_tool_add_error(result, "INTERNAL_ERROR", metadata_error); + } + else if (metadata_status == TINY_METADATA_PARSE_INVALID) + { + tiny_mcp_tool_add_error(result, "VALIDATION_ERROR", metadata_error); + } else { status = tiny_store_write(context->store, &input, &written); diff --git a/tests/mcp_protocol_test.py b/tests/mcp_protocol_test.py index f5a5c86..c206515 100644 --- a/tests/mcp_protocol_test.py +++ b/tests/mcp_protocol_test.py @@ -109,6 +109,7 @@ def main() -> None: data_dir = Path(tempfile.mkdtemp(prefix="tiny-convmem-p0-")) server = Server(args.server.resolve(), data_dir) next_id = 1 + expected_session_count = 0 try: response = server.send( request( @@ -200,6 +201,9 @@ def main() -> None: assert tools["search_messages"]["inputSchema"]["properties"]["limit"]["type"] == "integer" assert tools["delete_message"]["inputSchema"]["required"] == ["message_id"] assert tools["update_message"]["inputSchema"]["required"] == ["message_id"] + empty_health = tool_payload(tool_call(server, next_id, "get_system_health", {})) + next_id += 1 + assert empty_health["session_count"] == 0 message_ids: list[str] = [] for index in range(20): @@ -213,6 +217,71 @@ def main() -> None: ) next_id += 1 message_ids.append(payload["message"]["message_id"]) + + metadata_regression = tool_payload( + tool_call( + server, + next_id, + "write_message", + { + "session_id": "metadata-regression", + "role": "assistant", + "content": "metadata combination", + "metadata_json": json.dumps( + { + "tags": ["regression"], + "memory_tier": "long", + "importance": 90, + "summary": "valid metadata combination", + } + ), + }, + ) + )["message"] + next_id += 1 + assert metadata_regression["metadata"]["tags"] == ["regression"] + assert metadata_regression["metadata"]["memory_tier"] == "long" + assert metadata_regression["metadata"]["importance"] == 90 + assert metadata_regression["metadata"]["summary"] == "valid metadata combination" + + invalid_metadata_cases = [ + (json.dumps({"unexpected": True}), "unknown field"), + (json.dumps({"memory_tier": "archive"}), "memory_tier"), + (json.dumps({"importance": 101}), "importance"), + (json.dumps({"summary": "x" * 4097}), "summary"), + (json.dumps({"tags": [1]}), "tags"), + ("{", "JSON object"), + ] + for metadata_json, expected_diagnostic in invalid_metadata_cases: + write_error = tool_payload( + tool_call( + server, + next_id, + "write_message", + { + "session_id": "metadata-invalid", + "role": "user", + "content": "must not be written", + "metadata_json": metadata_json, + }, + ), + True, + ) + next_id += 1 + assert write_error["error"]["code"] == "VALIDATION_ERROR" + assert expected_diagnostic in write_error["error"]["message"] + update_error = tool_payload( + tool_call( + server, + next_id, + "update_message", + {"message_id": message_ids[0], "metadata_json": metadata_json}, + ), + True, + ) + next_id += 1 + assert update_error["error"]["code"] == "VALIDATION_ERROR" + assert expected_diagnostic in update_error["error"]["message"] updated = tool_payload( tool_call( server, @@ -273,6 +342,63 @@ def main() -> None: io_after_views = tool_payload(tool_call(server, next_id, "get_system_health", {}))["io"] next_id += 1 assert io_after_views == io_before + + paged_session_ids = ["session-page-a", "session-page-b", "s" * 256] + for session_id in paged_session_ids: + tool_payload( + tool_call( + server, + next_id, + "write_message", + {"session_id": session_id, "role": "user", "content": "session page item"}, + ) + ) + next_id += 1 + complete_sessions = tool_payload( + tool_call(server, next_id, "read_messages", {"view": "sessions", "limit": 1000}) + )["sessions"] + next_id += 1 + expected_session_ids = [item["session_id"] for item in complete_sessions] + traversed_session_ids: list[str] = [] + session_cursor: str | None = None + first_session_cursor: str | None = None + while True: + arguments: dict[str, Any] = {"view": "sessions", "limit": 1} + if session_cursor: + arguments["cursor"] = session_cursor + session_page = tool_payload( + tool_call(server, next_id, "read_messages", arguments) + ) + next_id += 1 + assert len(session_page["sessions"]) == 1 + traversed_session_ids.append(session_page["sessions"][0]["session_id"]) + session_cursor = session_page.get("nextCursor") + if first_session_cursor is None: + first_session_cursor = session_cursor + if not session_cursor: + break + assert len(traversed_session_ids) <= len(expected_session_ids) + assert traversed_session_ids == expected_session_ids + assert first_session_cursor + tampered_cursor = first_session_cursor[:-1] + ( + "A" if first_session_cursor[-1] != "A" else "B" + ) + assert tool_payload( + tool_call( + server, + next_id, + "read_messages", + {"view": "sessions", "limit": 1, "cursor": tampered_cursor}, + ), + True, + )["error"]["code"] == "VALIDATION_ERROR" + next_id += 1 + health_with_sessions = tool_payload( + tool_call(server, next_id, "get_system_health", {}) + ) + next_id += 1 + expected_session_count = len(expected_session_ids) + assert health_with_sessions["session_count"] == expected_session_count relation_message = tool_payload( tool_call( server, @@ -429,6 +555,46 @@ def main() -> None: next_id += 1 assert 0 < len(large_page["history"]) < 6 and large_page.get("nextCursor") assert server.last_response_size <= 8 * 1024 * 1024 + + large_session_ids = [f"large-session-{index}" for index in range(5)] + for session_id in large_session_ids: + tool_payload( + tool_call( + server, + next_id, + "write_message", + {"session_id": session_id, "role": "user", "content": large_content}, + ) + ) + next_id += 1 + budget_session_ids: list[str] = [] + budget_cursor: str | None = None + budget_page_count = 0 + while True: + arguments = {"view": "sessions", "limit": 1000} + if budget_cursor: + arguments["cursor"] = budget_cursor + budget_page = tool_payload( + tool_call(server, next_id, "read_messages", arguments) + ) + next_id += 1 + budget_page_count += 1 + assert budget_page["sessions"] + assert server.last_response_size <= 8 * 1024 * 1024 + budget_session_ids.extend(item["session_id"] for item in budget_page["sessions"]) + budget_cursor = budget_page.get("nextCursor") + if not budget_cursor: + break + assert budget_page_count <= 10 + assert budget_page_count > 1 + assert len(budget_session_ids) == len(set(budget_session_ids)) + assert set(large_session_ids).issubset(budget_session_ids) + health_after_budget = tool_payload( + tool_call(server, next_id, "get_system_health", {}) + ) + next_id += 1 + expected_session_count = len(budget_session_ids) + assert health_after_budget["session_count"] == expected_session_count assert tool_payload( tool_call( server, @@ -559,6 +725,7 @@ def main() -> None: ) next_id += 1 assert resumed_health["status"] == "OK" + assert resumed_health["session_count"] == expected_session_count finally: restarted.close() finally: @@ -567,4 +734,3 @@ def main() -> None: if __name__ == "__main__": main() - -- cgit v1.2.3