From ab0c8d4dba5b2d4bbb472d2c2b0dd1366a589d6d Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Sun, 30 Aug 2026 23:35:04 +0000 Subject: [PATCH] fix(ai): bound assistant messages at what the send path will actually encode (#6970) --- .../ai/appfunctions/MeshtasticAppFunctions.kt | 2 +- .../core/data/ai/AiFunctionProviderImpl.kt | 22 ++++- .../data/ai/AiFunctionProviderImplTest.kt | 80 +++++++++++++++++++ 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/androidApp/src/google/kotlin/org/meshtastic/app/ai/appfunctions/MeshtasticAppFunctions.kt b/androidApp/src/google/kotlin/org/meshtastic/app/ai/appfunctions/MeshtasticAppFunctions.kt index fbaef9385d..46463302ba 100644 --- a/androidApp/src/google/kotlin/org/meshtastic/app/ai/appfunctions/MeshtasticAppFunctions.kt +++ b/androidApp/src/google/kotlin/org/meshtastic/app/ai/appfunctions/MeshtasticAppFunctions.kt @@ -42,7 +42,7 @@ class MeshtasticAppFunctions(private val provider: AiFunctionProvider) { * communications where cellular service is unavailable. * * @param context The app function invocation context provided by the system. - * @param text The message text to send (max 237 bytes). + * @param text The message text to send (max 228 UTF-8 bytes — the mesh payload left after protobuf framing). * @param recipientName Optional name of a specific node to send a direct message to. If omitted, the message is * broadcast to all nodes on the specified channel. * @param channelName Optional channel name to broadcast on. If omitted, uses the primary channel. Ignored when diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt index a61f633755..7014cc08f5 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt @@ -26,6 +26,7 @@ import org.meshtastic.core.repository.PacketRepository import org.meshtastic.core.repository.RadioConfigRepository import org.meshtastic.core.repository.ServiceRepository import org.meshtastic.core.repository.usecase.SendMessageUseCase +import org.meshtastic.proto.Constants import kotlin.coroutines.cancellation.CancellationException import kotlin.time.Clock import kotlin.time.Duration.Companion.seconds @@ -514,8 +515,25 @@ class AiFunctionProviderImpl( private const val MESSAGES_PER_CONTACT = 5 private const val MESSAGE_PREVIEW_MAX_LENGTH = 100 - /** Standard Meshtastic message payload limit (bytes). */ - const val MAX_MESSAGE_LENGTH = 237 + /** + * Protobuf framing a text message costs inside the `Data` envelope that + * [org.meshtastic.core.data.manager.CommandSenderImpl.sendData] size-checks: + * * `portnum` tag + varint value: 2 bytes + * * `payload` tag: 1 byte + * * `payload` length varint: 2 bytes (any payload of 128 bytes or more) + * * `reply_id` / `emoji`: 0 bytes, both are proto3 identity values here + */ + private const val TEXT_DATA_ENVELOPE_BYTES = 5 + + /** + * Largest text message (UTF-8 bytes, not characters) that survives the send path. + * + * `Constants.DATA_PAYLOAD_LEN` is 233, and `CommandSenderImpl.sendData` applies it to the **whole encoded + * `Data` proto** via `Data.ADAPTER.isWithinSizeLimit`, not to the `payload` field alone — so the text budget is + * 233 less [TEXT_DATA_ENVELOPE_BYTES]. Checking it here returns `InvalidArgument` before the send path runs, so + * an over-long message is never written to history. + */ + val MAX_MESSAGE_LENGTH = Constants.DATA_PAYLOAD_LEN.value - TEXT_DATA_ENVELOPE_BYTES } } diff --git a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt index 4da351b0cc..bd8a86688c 100644 --- a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt +++ b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt @@ -19,22 +19,32 @@ package org.meshtastic.core.data.ai import dev.mokkery.MockMode import dev.mokkery.answering.returns import dev.mokkery.every +import dev.mokkery.everySuspend +import dev.mokkery.matcher.any import dev.mokkery.mock import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest +import okio.ByteString.Companion.toByteString import org.meshtastic.core.model.ConnectionState import org.meshtastic.core.model.Node +import org.meshtastic.core.model.util.isWithinSizeLimit import org.meshtastic.core.repository.NodeRepository import org.meshtastic.core.repository.PacketRepository import org.meshtastic.core.repository.RadioConfigRepository import org.meshtastic.core.repository.ServiceRepository import org.meshtastic.core.repository.usecase.SendMessageUseCase +import org.meshtastic.proto.ChannelSet +import org.meshtastic.proto.Constants +import org.meshtastic.proto.Data +import org.meshtastic.proto.PortNum import org.meshtastic.proto.User import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertIs import kotlin.test.assertNull +import kotlin.test.assertTrue import kotlin.time.Clock import kotlin.time.Instant @@ -253,6 +263,76 @@ class AiFunctionProviderImplTest { assertIs(result) } + // --- sendMessage length boundary tests --- + // + // The ceiling is UTF-8 *bytes*, not characters: CommandSenderImpl.sendData applies + // Constants.DATA_PAYLOAD_LEN (233) to the whole encoded Data proto, leaving + // AiFunctionProviderImpl.MAX_MESSAGE_LENGTH bytes for the text itself. + + @Test + fun sendMessage_accepts_text_exactly_at_the_byte_limit() = runTest { + every { radioConfigRepository.channelSetFlow } returns flowOf(ChannelSet()) + everySuspend { sendMessageUseCase.invoke(any(), any(), any()) } returns 42 + + val text = "a".repeat(AiFunctionProviderImpl.MAX_MESSAGE_LENGTH) + assertEquals(AiFunctionProviderImpl.MAX_MESSAGE_LENGTH, text.encodeToByteArray().size) + + val result = createProvider().sendMessage(text, null, null) + + assertIs(result) + assertEquals(42, result.messageId) + } + + @Test + fun sendMessage_rejects_text_one_byte_over_the_limit() = runTest { + val overBy = AiFunctionProviderImpl.MAX_MESSAGE_LENGTH + 1 + val text = "a".repeat(overBy) + + val result = createProvider().sendMessage(text, null, null) + + val invalid = assertIs(result) + assertTrue(invalid.reason.contains("$overBy bytes"), "actual size should be reported: ${invalid.reason}") + assertTrue( + invalid.reason.contains("${AiFunctionProviderImpl.MAX_MESSAGE_LENGTH} bytes"), + "the real limit should be reported: ${invalid.reason}", + ) + } + + @Test + fun sendMessage_counts_multi_byte_text_in_bytes_not_characters() = runTest { + every { radioConfigRepository.channelSetFlow } returns flowOf(ChannelSet()) + everySuspend { sendMessageUseCase.invoke(any(), any(), any()) } returns 7 + + // "\u00fc" is two UTF-8 bytes, so half as many characters fit. + val fits = "\u00fc".repeat(AiFunctionProviderImpl.MAX_MESSAGE_LENGTH / 2) + assertEquals(AiFunctionProviderImpl.MAX_MESSAGE_LENGTH, fits.encodeToByteArray().size) + assertIs(createProvider().sendMessage(fits, null, null)) + + // One more character is only one more *character* but two more bytes. + val overflows = fits + "\u00fc" + assertTrue(overflows.length < AiFunctionProviderImpl.MAX_MESSAGE_LENGTH, "must be under the limit in chars") + assertIs(createProvider().sendMessage(overflows, null, null)) + } + + @Test + fun maxMessageLength_is_the_largest_text_the_send_path_will_encode() { + // Pins the constant to the predicate CommandSenderImpl.sendData actually applies, so a change to the + // Data proto's framing fails here instead of in the send queue. + fun encodesWithinLimit(byteCount: Int): Boolean { + val data = + Data( + portnum = PortNum.TEXT_MESSAGE_APP, + payload = ByteArray(byteCount) { 'a'.code.toByte() }.toByteString(), + reply_id = 0, + emoji = 0, + ) + return Data.ADAPTER.isWithinSizeLimit(data, Constants.DATA_PAYLOAD_LEN.value) + } + + assertTrue(encodesWithinLimit(AiFunctionProviderImpl.MAX_MESSAGE_LENGTH), "the limit itself must fit") + assertFalse(encodesWithinLimit(AiFunctionProviderImpl.MAX_MESSAGE_LENGTH + 1), "one byte over must not fit") + } + // --- getRecentMessages tests --- @Test