From 944390bec10c62cd8e9bd55e0d9e7ad84d9423fc Mon Sep 17 00:00:00 2001 From: m57 <41645758+Hy4ri@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:51:11 +0300 Subject: [PATCH] chore: promote dev to main (#1455) Co-authored-by: AlexDev_ <56083016+alexdev03@users.noreply.github.com> --- .../chat/components/ComposerImagePasteTest.kt | 257 ++++++++++ .../components/ComposerInteractionTest.kt | 12 +- .../chat/components/ComposerReadinessTest.kt | 17 +- .../components/PendingSendRecoveryTest.kt | 2 +- .../chat/components/VoiceNoteGestureTest.kt | 10 +- .../com/m57/hermescontrol/HermesControlApp.kt | 5 + .../data/local/ChatMessageMapper.kt | 3 +- .../data/model/SessionMessagesResponse.kt | 36 +- .../m57/hermescontrol/data/ws/EventParser.kt | 2 +- .../com/m57/hermescontrol/data/ws/WsEvent.kt | 1 + .../data/ws/contract/SessionPrompt.kt | 12 + .../notification/NotificationReplyReceiver.kt | 2 + .../ui/bots/group/GroupChatViewModel.kt | 3 +- .../ui/chat/ChatAttachmentTarget.kt | 10 + .../hermescontrol/ui/chat/ChatListEntries.kt | 5 +- .../ui/chat/ChatPastedImageStore.kt | 175 +++++++ .../m57/hermescontrol/ui/chat/ChatScreen.kt | 30 +- .../ui/chat/ChatSearchController.kt | 3 +- .../hermescontrol/ui/chat/ChatSendStore.kt | 19 + .../ui/chat/ChatServerMessageMapper.kt | 6 +- .../ui/chat/ChatTranscriptMerge.kt | 46 +- .../hermescontrol/ui/chat/ChatViewModel.kt | 137 ++++-- .../ui/chat/ChatWsEventReducer.kt | 92 +++- .../hermescontrol/ui/chat/GatewayNotice.kt | 32 ++ .../ui/chat/SlashCommandDispatcher.kt | 2 +- .../m57/hermescontrol/ui/chat/UserBubble.kt | 72 ++- .../ui/chat/UserImageAttachments.kt | 63 +++ .../ui/chat/components/ChatComposer.kt | 68 ++- .../ui/chat/components/ChatComposerState.kt | 31 ++ .../components/ChatImagePasteController.kt | 195 ++++++++ .../ui/chat/components/ChatMediaLaunchers.kt | 18 +- .../ui/chat/fullbleed/FullBleedChatList.kt | 19 +- .../ui/chat/fullbleed/FullBleedTurns.kt | 9 + app/src/main/res/values/chat_image_paste.xml | 8 + .../data/model/SessionMessageMultipartTest.kt | 233 +++++++++ .../data/ws/GatewayContractFramesTest.kt | 22 +- .../data/ws/GatewayContractTest.kt | 2 +- .../ws/HermesWsClientSessionControlTest.kt | 28 +- .../data/ws/HermesWsClientTest.kt | 85 ++-- .../data/ws/HermesWsClientTypedCallTest.kt | 38 +- .../data/ws/HermesWsClientTypedSendTest.kt | 36 +- .../data/ws/MockWebSocketCleanup.kt | 56 +++ .../ws/MockWebSocketCleanupListenerTest.kt | 113 +++++ .../data/ws/MockWebSocketCleanupTest.kt | 104 ++++ .../ws/contract/SessionPromptTypesTest.kt | 42 ++ .../NotificationReplyReceiverTest.kt | 8 +- .../ui/chat/ChatLiveOrderingRegressionTest.kt | 464 ++++++++++++++++++ .../ui/chat/ChatPastedImageStoreTest.kt | 253 ++++++++++ .../ui/chat/ChatSendStoreTest.kt | 23 + .../ui/chat/ChatViewModelTest.kt | 120 ++++- .../ui/chat/FullBleedTurnsTest.kt | 23 + .../ui/chat/GatewayNoticeTest.kt | 233 +++++++++ .../ui/chat/SlashCommandDispatchRpcTest.kt | 185 +++++-- .../ui/chat/UserImageHydrationTest.kt | 355 ++++++++++++++ .../ChatImagePasteControllerTest.kt | 342 +++++++++++++ .../chat/fullbleed/TranscriptUiStateTest.kt | 51 ++ 56 files changed, 3914 insertions(+), 304 deletions(-) create mode 100644 app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerImagePasteTest.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatAttachmentTarget.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatPastedImageStore.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/GatewayNotice.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/UserImageAttachments.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatComposerState.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatImagePasteController.kt create mode 100644 app/src/main/res/values/chat_image_paste.xml create mode 100644 app/src/test/java/com/m57/hermescontrol/data/model/SessionMessageMultipartTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanup.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanupListenerTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanupTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatLiveOrderingRegressionTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPastedImageStoreTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/GatewayNoticeTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/UserImageHydrationTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/components/ChatImagePasteControllerTest.kt diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerImagePasteTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerImagePasteTest.kt new file mode 100644 index 00000000..348b7851 --- /dev/null +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerImagePasteTest.kt @@ -0,0 +1,257 @@ +package com.m57.hermescontrol.ui.chat.components + +import android.content.ClipData +import android.content.ClipDescription +import android.content.ClipboardManager +import android.graphics.Bitmap +import android.net.Uri +import android.view.inputmethod.EditorInfo +import android.view.inputmethod.InputConnection +import android.view.inputmethod.InputContentInfo +import androidx.activity.ComponentActivity +import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.text.input.TextFieldState +import androidx.compose.runtime.mutableStateListOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.ui.ExperimentalComposeUiApi +import androidx.compose.ui.platform.InterceptPlatformTextInput +import androidx.compose.ui.semantics.SemanticsActions +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performSemanticsAction +import androidx.compose.ui.text.TextRange +import androidx.core.content.FileProvider +import androidx.test.platform.app.InstrumentationRegistry +import com.m57.hermescontrol.data.model.Attachment +import com.m57.hermescontrol.data.ws.CommandCatalog +import com.m57.hermescontrol.ui.chat.ChatAttachmentTarget +import com.m57.hermescontrol.ui.chat.ChatPastedImageStore +import kotlinx.coroutines.awaitCancellation +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import java.io.File +import java.util.UUID + +/** Real BasicTextField paste and InputConnection entry points; no direct listener invocation. */ +@OptIn(ExperimentalFoundationApi::class, ExperimentalComposeUiApi::class) +class ComposerImagePasteTest { + @get:Rule val compose = createAndroidComposeRule() + private val context get() = InstrumentationRegistry.getInstrumentation().targetContext + private val attachments = mutableStateListOf() + private val errors = mutableListOf() + private val ownedFiles = mutableListOf() + private val input = TextFieldState("keep draft", TextRange(1, 3)) + private var sends = 0 + private var connection: InputConnection? = null + private var editorInfo: EditorInfo? = null + private lateinit var controller: ChatImagePasteController + + private fun prepareComposer(interceptIme: Boolean = false) { + compose.setContent { + val scope = rememberCoroutineScope() + controller = + remember { + ChatImagePasteController( + scope = scope, + importImage = ChatPastedImageStore(context)::importImage, + captureTarget = { ChatAttachmentTarget("test", 1, "http://test.local", null, null) }, + commit = { _, images -> + attachments.addAll(images) + true + }, + onError = errors::add, + isImageUri = { uri -> + context.contentResolver.getType(uri)?.startsWith("image/", ignoreCase = true) == true + }, + ) + } + InterceptPlatformTextInput(interceptor = { request, next -> + if (interceptIme) { + val info = EditorInfo() + connection = request.createInputConnection(info) + editorInfo = info + awaitCancellation() + } else { + next.startInputMethod(request) + } + }) { + ChatInputBar( + inputState = input, + onSend = { sends++ }, + onMicTap = {}, + isListening = false, + isAgentTyping = false, + isConnected = true, + isSessionReady = true, + commandCatalog = CommandCatalog(), + pendingAttachments = attachments, + receiveContentListener = controller, + isReceivingContent = controller.isReceiving, + ) + } + } + compose.onNodeWithTag("chat_input").performClick() + compose.runOnIdle { input.edit { selection = TextRange(1, 3) } } + } + + private fun imageUri(): Uri { + val file = File(context.cacheDir, "clipboard-test-${UUID.randomUUID()}.png") + ownedFiles.add(file) + val bitmap = Bitmap.createBitmap(2, 2, Bitmap.Config.ARGB_8888) + try { + file.outputStream().use { assertTrue(bitmap.compress(Bitmap.CompressFormat.PNG, 100, it)) } + } finally { + bitmap.recycle() + } + return FileProvider.getUriForFile(context, "${context.packageName}.fileprovider", file) + } + + private fun pdfUri(): Uri { + val file = File(context.cacheDir, "clipboard-test-${UUID.randomUUID()}.pdf") + ownedFiles.add(file) + file.writeText("%PDF-1.4\nfixture\n") + return FileProvider.getUriForFile(context, "${context.packageName}.fileprovider", file) + } + + private fun paste(clip: ClipData) { + compose.runOnIdle { + context.getSystemService(ClipboardManager::class.java).setPrimaryClip(clip) + } + compose.onNodeWithTag("chat_input").performSemanticsAction(SemanticsActions.PasteText) { it() } + } + + private fun assertImported(source: Uri) { + compose.waitUntil(timeoutMillis = 10_000) { attachments.size == 1 && !controller.isReceiving } + compose.runOnIdle { + assertEquals("keep draft", input.text.toString()) + assertEquals(TextRange(1, 3), input.selection) + assertEquals(0, sends) + assertTrue(errors.isEmpty()) + assertNotEquals(source.toString(), attachments.single().uri) + val copy = File(context.cacheDir, "pasted_images/${attachments.single().name}") + assertTrue(copy.isFile) + assertEquals(copy.length(), attachments.single().size) + } + } + + @Test + fun clipboardImage_becomesPrivateAttachmentWithoutReplacingSelectionOrSending() { + prepareComposer() + val source = imageUri() + paste(ClipData.newUri(context.contentResolver, "test image", source)) + assertImported(source) + // Clipboard replacement cannot revoke access to the independently owned chip. + compose.runOnIdle { + context + .getSystemService(ClipboardManager::class.java) + .setPrimaryClip(ClipData.newPlainText("replacement", "different")) + assertTrue(File(context.cacheDir, "pasted_images/${attachments.single().name}").isFile) + } + } + + @Test + fun imeCommitContent_usesRealInputConnectionAndPreservesDraft() { + prepareComposer(interceptIme = true) + compose.waitUntil(timeoutMillis = 5_000) { connection != null } + val source = imageUri() + compose.runOnIdle { + assertTrue( + editorInfo!!.contentMimeTypes.orEmpty().any { ClipDescription.compareMimeTypes("image/png", it) }, + ) + assertTrue( + connection!!.commitContent( + InputContentInfo(source, ClipDescription("image", arrayOf("image/png")), null), + 0, + null, + ), + ) + } + assertImported(source) + } + + @Test + fun mixedClipboard_keepsTextItemsOnTheNormalPastePath() { + prepareComposer() + val source = imageUri() + val clip = ClipData(ClipDescription("mixed", arrayOf("image/png", "text/plain")), ClipData.Item("hello")) + clip.addItem(ClipData.Item(source)) + paste(clip) + compose.waitUntil(timeoutMillis = 10_000) { attachments.size == 1 && !controller.isReceiving } + compose.runOnIdle { + assertEquals("khellop draft", input.text.toString()) + assertFalse(input.text.contains("content://")) + assertEquals(0, sends) + assertTrue(errors.isEmpty()) + } + } + + @Test + fun dualRepresentationItem_preservesTextPasteAndImportsImage() { + prepareComposer() + val source = imageUri() + val clip = + ClipData( + ClipDescription("dual", arrayOf("image/png", "text/plain")), + ClipData.Item("hello", null, null, source), + ) + paste(clip) + compose.waitUntil(timeoutMillis = 10_000) { attachments.size == 1 && !controller.isReceiving } + compose.runOnIdle { + assertEquals("khellop draft", input.text.toString()) + assertEquals("image/png", attachments.single().mimeType) + assertNotEquals(source.toString(), attachments.single().uri) + assertTrue(File(context.cacheDir, "pasted_images/${attachments.single().name}").isFile) + assertTrue(errors.isEmpty()) + assertEquals(0, sends) + } + } + + @Test + fun mixedClipboard_importsImageDespitePdfAndPreservesText() { + prepareComposer() + val image = imageUri() + val pdf = pdfUri() + val clip = + ClipData( + ClipDescription("mixed", arrayOf("image/png", "application/pdf", "text/plain")), + ClipData.Item("hello"), + ) + clip.addItem(ClipData.Item(image)) + clip.addItem(ClipData.Item(pdf)) + paste(clip) + compose.waitUntil(timeoutMillis = 10_000) { attachments.size == 1 && !controller.isReceiving } + compose.runOnIdle { + assertEquals("khellop draft", input.text.toString()) + assertEquals("image/png", attachments.single().mimeType) + assertNotEquals(image.toString(), attachments.single().uri) + assertTrue(File(context.cacheDir, "pasted_images/${attachments.single().name}").isFile) + assertTrue(errors.isEmpty()) + assertEquals(0, sends) + } + } + + @Test + fun plainTextPaste_stillReplacesOnlyTheSelection() { + prepareComposer() + paste(ClipData.newPlainText("text", "hello")) + compose.runOnIdle { + assertEquals("khellop draft", input.text.toString()) + assertTrue(attachments.isEmpty()) + assertEquals(0, sends) + } + } + + @After + fun cleanupOnlyTestOwnedFiles() { + if (::controller.isInitialized) compose.runOnIdle { controller.close() } + ownedFiles.forEach { it.delete() } + attachments.forEach { File(context.cacheDir, "pasted_images/${it.name}").delete() } + } +} diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerInteractionTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerInteractionTest.kt index cdda65ee..640b541c 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerInteractionTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerInteractionTest.kt @@ -1,8 +1,11 @@ package com.m57.hermescontrol.ui.chat.components import androidx.activity.ComponentActivity +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.width +import androidx.compose.foundation.text.input.clearText +import androidx.compose.foundation.text.input.rememberTextFieldState import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.getValue @@ -23,7 +26,6 @@ import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTextInput import androidx.compose.ui.test.performTouchInput import androidx.compose.ui.test.swipeLeft -import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.LayoutDirection import androidx.compose.ui.unit.dp @@ -58,6 +60,7 @@ class ComposerInteractionTest { private var fileTaps = 0 /** Renders the real input bar with live text and a mic that toggles like ChatMediaLaunchers. */ + @OptIn(ExperimentalFoundationApi::class) private fun setComposer( reasoningLevel: String? = "medium", model: String = "openai/gpt-5.5", @@ -67,18 +70,17 @@ class ComposerInteractionTest { showModelProvider: Boolean = false, ) { composeTestRule.setContent { - var input by remember { mutableStateOf(TextFieldValue("")) } + val input = rememberTextFieldState() var listening by remember { mutableStateOf(false) } var currentModel by remember { mutableStateOf(model) } CompositionLocalProvider(LocalLayoutDirection provides layoutDirection) { val composer: @Composable () -> Unit = { ChatInputBar( - inputFieldValue = input, - onInputChange = { input = it }, + inputState = input, onSend = { sends++ - input = TextFieldValue("") + input.clearText() }, onMicTap = { micTaps++ diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerReadinessTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerReadinessTest.kt index cfd5c863..cdc1094b 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerReadinessTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ComposerReadinessTest.kt @@ -1,9 +1,8 @@ package com.m57.hermescontrol.ui.chat.components -import androidx.compose.runtime.getValue +import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.text.input.rememberTextFieldState import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue import androidx.compose.ui.test.assertIsEnabled import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.assertTextEquals @@ -11,12 +10,12 @@ import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTextInput -import androidx.compose.ui.text.input.TextFieldValue import com.m57.hermescontrol.data.ws.CommandCatalog import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test +@OptIn(ExperimentalFoundationApi::class) class ComposerReadinessTest { @get:Rule val compose = createComposeRule() @@ -26,10 +25,9 @@ class ComposerReadinessTest { val ready = mutableStateOf(false) var sends = 0 compose.setContent { - var input by remember { mutableStateOf(TextFieldValue("")) } + val input = rememberTextFieldState() ChatInputBar( - inputFieldValue = input, - onInputChange = { input = it }, + inputState = input, onSend = { sends++ }, onMicTap = {}, isListening = false, @@ -64,10 +62,9 @@ class ComposerReadinessTest { @Test fun emptyDraftWhileStreamingMakesStopThePrimaryAction() { compose.setContent { - var input by remember { mutableStateOf(TextFieldValue("")) } + val input = rememberTextFieldState() ChatInputBar( - inputFieldValue = input, - onInputChange = { input = it }, + inputState = input, onSend = {}, onMicTap = {}, isListening = false, diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/PendingSendRecoveryTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/PendingSendRecoveryTest.kt index 43761748..2f4b3363 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/PendingSendRecoveryTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/PendingSendRecoveryTest.kt @@ -130,7 +130,7 @@ class PendingSendRecoveryTest { .boundsInRoot.height > 0 } compose.onNodeWithContentDescription("photo.png").assertIsDisplayed() - compose.onNodeWithText("Accepted by server").assertIsDisplayed() + compose.onNodeWithContentDescription("Accepted by server").assertIsDisplayed() compose.onNodeWithTag("pending_send_recovery").assertDoesNotExist() } diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/VoiceNoteGestureTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/VoiceNoteGestureTest.kt index 86f7f60b..72d50f63 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/VoiceNoteGestureTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/VoiceNoteGestureTest.kt @@ -3,9 +3,11 @@ package com.m57.hermescontrol.ui.chat.components import android.content.Context import android.graphics.Bitmap import androidx.activity.ComponentActivity +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.text.input.rememberTextFieldState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -19,6 +21,7 @@ import androidx.compose.ui.test.SemanticsMatcher import androidx.compose.ui.test.assert import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsFocused +import androidx.compose.ui.test.assertTextEquals import androidx.compose.ui.test.captureToImage import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithTag @@ -26,7 +29,6 @@ import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTouchInput -import androidx.compose.ui.text.input.TextFieldValue import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.MediumTest import com.m57.hermescontrol.data.ws.CommandCatalog @@ -52,6 +54,7 @@ import java.io.File */ @RunWith(AndroidJUnit4::class) @MediumTest +@OptIn(ExperimentalFoundationApi::class) class VoiceNoteGestureTest { @get:Rule val composeTestRule = createAndroidComposeRule() @@ -88,8 +91,7 @@ class VoiceNoteGestureTest { Column(modifier = Modifier.fillMaxSize()) { Spacer(modifier = Modifier.weight(1f)) ChatInputBar( - inputFieldValue = TextFieldValue(draft), - onInputChange = {}, + inputState = rememberTextFieldState(initialText = draft), onSend = { sendCount++ }, onMicTap = { micTaps++ }, isListening = false, @@ -275,6 +277,8 @@ class VoiceNoteGestureTest { assertEquals("release must submit the note", 1, holdEnds) assertEquals("a stationary hold-release must not fire a tap", tapsBefore, micTaps) } + // The state-based input must retain the draft when the recording strip is removed. + composeTestRule.onNodeWithTag("chat_input").assertTextEquals("draft") } @Test diff --git a/app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt b/app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt index b9a33c3e..fe0ea4fe 100644 --- a/app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt +++ b/app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt @@ -15,6 +15,7 @@ import com.m57.hermescontrol.data.remote.NetworkMonitor import com.m57.hermescontrol.data.remote.OkHttpProvider import com.m57.hermescontrol.notification.TurnCorrelationTracker import com.m57.hermescontrol.ui.analytics.AnalyticsPreloader +import com.m57.hermescontrol.ui.chat.cleanStalePastedImages import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -28,6 +29,10 @@ class HermesControlApp : override fun onCreate() { super.onCreate() + val startupTimeMs = System.currentTimeMillis() + appScope.launch(Dispatchers.IO) { + cleanStalePastedImages(cacheDir, startupTimeMs) + } AuthManager.init(this) NetworkMonitor.init(this) SessionListCacheStore.init(this) diff --git a/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageMapper.kt b/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageMapper.kt index 5be0dd2d..4470fcdc 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageMapper.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageMapper.kt @@ -4,6 +4,7 @@ import com.m57.hermescontrol.ui.chat.ChatMessage import com.m57.hermescontrol.ui.chat.MessageProvenance import com.m57.hermescontrol.ui.chat.MessageRole import com.m57.hermescontrol.ui.chat.ToolStatus +import com.m57.hermescontrol.ui.chat.normalizedGatewayNotice /** * Converts between the Room [ChatMessageEntity] and the UI [ChatMessage]. @@ -44,7 +45,7 @@ fun ChatMessageEntity.toUiModel(): ChatMessage = messageProvenance = MessageProvenance.entries.firstOrNull { it.name == messageProvenance } ?: MessageProvenance.UNKNOWN, - ) + ).normalizedGatewayNotice() fun ChatMessage.toEntity(sessionId: String): ChatMessageEntity = ChatMessageEntity( diff --git a/app/src/main/java/com/m57/hermescontrol/data/model/SessionMessagesResponse.kt b/app/src/main/java/com/m57/hermescontrol/data/model/SessionMessagesResponse.kt index 1e0b6ca0..ba680914 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/model/SessionMessagesResponse.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/model/SessionMessagesResponse.kt @@ -3,6 +3,7 @@ package com.m57.hermescontrol.data.model import kotlinx.serialization.Serializable import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.doubleOrNull import kotlinx.serialization.json.intOrNull @@ -100,20 +101,10 @@ data class SessionMessage( } val contentText: String - get() = - when (content) { - is JsonPrimitive -> content.content - null -> "" - else -> content.toString() - } + get() = content?.transcriptText().orEmpty() val displayContentText: String? - get() = - when (display_content) { - is JsonPrimitive -> display_content.content - null -> null - else -> display_content.toString() - } + get() = display_content?.transcriptText() val reasoningText: String get() = @@ -160,4 +151,25 @@ data class SessionMessage( get() = tool_call_id.orEmpty() } +// #1432: model-facing multipart payloads contain inline image bytes, not displayable JSON. +private fun JsonElement.transcriptText(): String = + when (this) { + is JsonPrimitive -> { + content + } + + is JsonArray -> { + mapNotNull { part -> + val block = part as? JsonObject ?: return@mapNotNull null + val type = block["type"] + if (type != null && (type as? JsonPrimitive)?.content != "text") return@mapNotNull null + (block["text"] as? JsonPrimitive)?.takeIf { it.isString }?.content + }.joinToString("\n") + } + + else -> { + toString() + } + } + private val WHITESPACE = Regex("\\s+") diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt index f4d56a93..15ef6908 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt @@ -279,7 +279,7 @@ object EventParser { "status.update" -> { val status = payload?.get("status") as? String - WsEvent.StatusUpdate(status, payload) + WsEvent.StatusUpdate(status, payload, sessionId) } "error" -> { diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt index 0f519032..0c1cb4ba 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt @@ -205,6 +205,7 @@ sealed class WsEvent { data class StatusUpdate( val status: String?, val data: Map?, + val sessionId: String? = null, ) : WsEvent() data class SessionUpdated( diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/SessionPrompt.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/SessionPrompt.kt index 42dcbd03..03148816 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/SessionPrompt.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/SessionPrompt.kt @@ -11,9 +11,21 @@ data class SessionCreateParams( @SerialName("hidden") val hidden: Boolean? = null, ) +/** + * The `source` this client declares for every session it mints or resumes. + * + * Mobile presents the desktop surface, so it claims `"desktop"` on BOTH `session.create` and + * `session.resume`. The gateway resolves an omitted `source` from the host environment + * (`_resolve_session_source`), which on a headless backend is `"tui"`; a resumed runtime then + * reads as a surface switch and loses the `desktop_ui` toolset (#1450). One constant so the + * create and resume call sites cannot drift apart again. + */ +const val DESKTOP_SESSION_SOURCE = "desktop" + @Serializable data class SessionResumeParams( @SerialName("session_id") val sessionId: String, + @SerialName("source") val source: String? = null, @SerialName("omit_messages") val omitMessages: Boolean? = null, @SerialName("profile") val profile: String? = null, ) diff --git a/app/src/main/java/com/m57/hermescontrol/notification/NotificationReplyReceiver.kt b/app/src/main/java/com/m57/hermescontrol/notification/NotificationReplyReceiver.kt index c72a073e..d7e9fddb 100644 --- a/app/src/main/java/com/m57/hermescontrol/notification/NotificationReplyReceiver.kt +++ b/app/src/main/java/com/m57/hermescontrol/notification/NotificationReplyReceiver.kt @@ -11,6 +11,7 @@ import com.m57.hermescontrol.R import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.session.ActiveSessionHolder import com.m57.hermescontrol.data.ws.HermesWsClient +import com.m57.hermescontrol.data.ws.contract.DESKTOP_SESSION_SOURCE import com.m57.hermescontrol.data.ws.contract.PromptSubmitParams import com.m57.hermescontrol.data.ws.contract.RpcMethods import com.m57.hermescontrol.data.ws.contract.SessionResumeParams @@ -157,6 +158,7 @@ open class NotificationReplyReceiver : BroadcastReceiver() { val params = SessionResumeParams( sessionId = storedSessionId, + source = DESKTOP_SESSION_SOURCE, omitMessages = true, profile = profile, ) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/bots/group/GroupChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/bots/group/GroupChatViewModel.kt index 52940d82..482b2410 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/bots/group/GroupChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/bots/group/GroupChatViewModel.kt @@ -16,6 +16,7 @@ import com.m57.hermescontrol.data.remote.safeApiCall import com.m57.hermescontrol.data.session.ActiveSessionHolder import com.m57.hermescontrol.data.ws.HermesWsClient import com.m57.hermescontrol.data.ws.WsEvent +import com.m57.hermescontrol.data.ws.contract.DESKTOP_SESSION_SOURCE import com.m57.hermescontrol.data.ws.contract.ProfilesConfigureParams import com.m57.hermescontrol.data.ws.contract.ProfilesListParams import com.m57.hermescontrol.data.ws.contract.PromptSubmitParams @@ -564,7 +565,7 @@ class GroupChatViewModel( method = RpcMethods.SESSION_CREATE, params = SessionCreateParams( - source = "desktop", + source = DESKTOP_SESSION_SOURCE, profile = bot.name, title = title, hidden = true, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatAttachmentTarget.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatAttachmentTarget.kt new file mode 100644 index 00000000..48026e75 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatAttachmentTarget.kt @@ -0,0 +1,10 @@ +package com.m57.hermescontrol.ui.chat + +/** Captured before asynchronous clipboard access; never retarget a late image to a new chat. */ +internal data class ChatAttachmentTarget( + val sessionId: String, + val generation: Long, + val baseUrl: String, + val connectionProfileId: String?, + val agentProfileId: String?, +) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatListEntries.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatListEntries.kt index 69732b47..76005b13 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatListEntries.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatListEntries.kt @@ -12,6 +12,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp +import com.m57.hermescontrol.ui.chat.fullbleed.isHiddenTool /** * Every [TOOL_CALL_DIVIDER_INTERVAL]th tool call within a single turn renders a @@ -56,7 +57,7 @@ fun toolCallMilestones(messages: List): Map { toolCount = 0 } - message.role == MessageRole.TOOL -> { + message.role == MessageRole.TOOL && !message.isHiddenTool() -> { toolCount += 1 if (toolCount % TOOL_CALL_DIVIDER_INTERVAL == 0) { milestones[message.id] = toolCount @@ -79,7 +80,7 @@ fun toolCallMilestoneIndices(messages: List): Map { toolCount = 0 } - message.role == MessageRole.TOOL -> { + message.role == MessageRole.TOOL && !message.isHiddenTool() -> { toolCount += 1 if (toolCount % TOOL_CALL_DIVIDER_INTERVAL == 0) { milestones[index] = toolCount diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatPastedImageStore.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatPastedImageStore.kt new file mode 100644 index 00000000..024d251f --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatPastedImageStore.kt @@ -0,0 +1,175 @@ +package com.m57.hermescontrol.ui.chat + +import android.content.Context +import android.net.Uri +import android.webkit.MimeTypeMap +import androidx.core.content.FileProvider +import com.m57.hermescontrol.data.model.Attachment +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.withContext +import java.io.File +import java.io.FileOutputStream +import java.io.IOException +import java.io.InputStream +import java.util.UUID + +private const val PASTED_IMAGE_DIRECTORY = "pasted_images" +private const val PASTED_IMAGE_MAX_AGE_MS = 7L * 24 * 60 * 60 * 1000 + +/** + * Startup-only sweep (#1444). Keep recent files for transcript previews/retries; + * never run during a live session where an old attachment may still be uploading. + */ +internal fun cleanStalePastedImages( + cacheDir: File, + nowMs: Long = System.currentTimeMillis(), +) { + val directory = File(cacheDir, PASTED_IMAGE_DIRECTORY) + val files = runCatching { directory.listFiles() }.getOrNull() ?: return + val cutoff = nowMs - PASTED_IMAGE_MAX_AGE_MS + for (file in files) { + runCatching { + // Do not traverse subdirectories or touch any other cache namespace. + if (file.isFile && file.lastModified() < cutoff) file.delete() + } + } +} + +internal class PastedImageTooLargeException : IOException("Pasted image exceeds attachment limit") + +/** A private cache file owned by the caller after [ChatPastedImageStore.importImage] returns. */ +class PastedImage( + val attachment: Attachment, + val file: File, +) { + /** Call if the import cannot be queued (for example, after a session switch). */ + fun discard() { + file.delete() + } +} + +/** Explicit clipboard/IME image importer; it never observes the clipboard itself. */ +class ChatPastedImageStore( + private val context: Context, +) { + /** Returns null for unsupported, missing, or empty input; oversized streams fail explicitly. */ + suspend fun importImage( + uri: Uri, + maxBytes: Long = MAX_CHAT_ATTACHMENT_BYTES, + ): PastedImage? { + var pending: PastedImage? = null + var delivered = false + try { + val result = + withContext(Dispatchers.IO) { + if (uri.scheme != "content") return@withContext null + val resolver = context.contentResolver + val mimeType = + resolver + .getType(uri) + ?.takeIf { it.startsWith("image/", ignoreCase = true) && it.length > "image/".length } + ?: return@withContext null + val extension = + MimeTypeMap + .getSingleton() + .getExtensionFromMimeType(mimeType) + ?.takeIf { it.matches(Regex("[A-Za-z0-9]{1,10}")) } + ?: "img" + val input = resolver.openInputStream(uri) ?: return@withContext null + val activeContext = currentCoroutineContext() + val file = + stagePastedImage( + input = input, + directory = File(context.cacheDir, PASTED_IMAGE_DIRECTORY), + extension = extension, + maxBytes = maxBytes, + checkActive = { activeContext.ensureActive() }, + ) ?: return@withContext null + try { + activeContext.ensureActive() + val privateUri = + FileProvider.getUriForFile( + context, + "${context.packageName}.fileprovider", + file, + ) + PastedImage( + attachment = + Attachment( + uri = privateUri.toString(), + name = file.name, + mimeType = mimeType, + size = file.length(), + ), + file = file, + ).also { pending = it } + } catch (failure: Throwable) { + file.delete() + throw failure + } + } + delivered = true + return result + } finally { + // withContext may discard its completed IO result if the parent was cancelled + // during dispatch back; in that case ownership never reached the caller. + if (!delivered) pending?.discard() + } + } +} + +/** JVM-testable, exclusive-create bounded copy. Null means empty input. */ +internal fun stagePastedImage( + input: InputStream, + directory: File, + extension: String, + maxBytes: Long = MAX_CHAT_ATTACHMENT_BYTES, + checkActive: () -> Unit = {}, + newId: () -> String = { UUID.randomUUID().toString() }, +): File? { + require(maxBytes > 0) { "maxBytes must be positive" } + val effectiveMaxBytes = minOf(maxBytes, MAX_CHAT_ATTACHMENT_BYTES) + require(extension.matches(Regex("[A-Za-z0-9]{1,10}"))) { "Invalid extension" } + var owned: File? = null + var keep = false + try { + return input + .use source@{ source -> + if (!directory.isDirectory && !directory.mkdirs() && !directory.isDirectory) { + throw IOException("Cannot create pasted image cache directory") + } + val file = File(directory, "${UUID.fromString(newId())}.$extension") + if (!file.createNewFile()) throw IOException("Pasted image cache name already exists") + owned = file + var total = 0L + val buffer = ByteArray(DEFAULT_BUFFER_SIZE) + FileOutputStream(file).use { output -> + while (true) { + checkActive() + // Probe one byte beyond the limit without trusting provider-reported size. + val remaining = effectiveMaxBytes - total + val requested = if (remaining < buffer.size) (remaining + 1).toInt() else buffer.size + val count = source.read(buffer, 0, requested) + if (count < 0) break + if (count > remaining) throw PastedImageTooLargeException() + if (count == 0) { + val byte = source.read() + if (byte < 0) break + if (remaining == 0L) throw PastedImageTooLargeException() + output.write(byte) + total++ + continue + } + output.write(buffer, 0, count) + total += count + } + checkActive() + } + if (total == 0L) null else file + }.also { keep = it != null } + } finally { + if (!keep) owned?.delete() + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt index 986f8acc..964ea918 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt @@ -28,6 +28,8 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.text.input.clearText +import androidx.compose.foundation.text.input.rememberTextFieldState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.filled.Close @@ -70,7 +72,6 @@ import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.unit.dp import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleEventObserver @@ -118,9 +119,12 @@ import com.m57.hermescontrol.ui.chat.components.SpeechRequest import com.m57.hermescontrol.ui.chat.components.SpeechText import com.m57.hermescontrol.ui.chat.components.SubagentInspectionSheet import com.m57.hermescontrol.ui.chat.components.TaskProgressChip +import com.m57.hermescontrol.ui.chat.components.rememberChatImagePaste import com.m57.hermescontrol.ui.chat.components.rememberChatMediaLaunchers import com.m57.hermescontrol.ui.chat.components.rememberChatScrollController import com.m57.hermescontrol.ui.chat.components.rememberChatSpeech +import com.m57.hermescontrol.ui.chat.components.replaceComposerDraft +import com.m57.hermescontrol.ui.chat.components.restoreRejectedComposerDraft import com.m57.hermescontrol.ui.chat.components.shouldShowProgressChip import com.m57.hermescontrol.ui.chat.components.tailContentKey import com.m57.hermescontrol.ui.chat.fullbleed.FullBleedChatList @@ -338,20 +342,18 @@ fun ChatScreen( !timelineState.isHistorical && scrollController.showFab(displayedMessages.isNotEmpty()) } } - var inputFieldValue by rememberSaveable(stateSaver = TextFieldValue.Saver) { - mutableStateOf(TextFieldValue("")) - } + val inputState = rememberTextFieldState() LaunchedEffect(state.pendingPrefillText) { val prefill = state.pendingPrefillText if (prefill != null) { - inputFieldValue = ChatInputPolicy.commandFieldValue(prefill) + inputState.replaceComposerDraft(prefill) viewModel.consumePendingPrefill() } } LaunchedEffect(state.composerTextToRestore) { state.composerTextToRestore?.let { text -> - inputFieldValue = ChatInputPolicy.restoreRejectedText(text, inputFieldValue) + inputState.restoreRejectedComposerDraft(text) viewModel.consumeComposerTextRestore() } } @@ -364,6 +366,10 @@ fun ChatScreen( } var viewingImage by rememberSaveable { mutableStateOf(null) } val snackbarHostState = remember { SnackbarHostState() } + val imagePaste = + rememberChatImagePaste(viewModel) { message -> + scrollScope.launch { snackbarHostState.showSnackbar(message) } + } val launchExternalActivity: (() -> Unit) -> Unit = { launch -> ExternalActivityLifecycleGuard.launchExternalActivity( acquireConnectionLease = HermesWsClient::acquireExternalActivityConnectionLease, @@ -411,8 +417,7 @@ fun ChatScreen( val mediaLaunchers = rememberChatMediaLaunchers( - inputFieldValue = inputFieldValue, - onInputFieldValueChange = { inputFieldValue = it }, + inputState = inputState, onAddAttachment = { uri, name, mimeType, size -> viewModel.addAttachment(uri, name, mimeType, size) }, @@ -865,11 +870,12 @@ fun ChatScreen( } ChatInputBar( - inputFieldValue = inputFieldValue, - onInputChange = { inputFieldValue = it }, + inputState = inputState, + receiveContentListener = imagePaste, + isReceivingContent = imagePaste.isReceiving, onSend = { - if (viewModel.sendMessage(inputFieldValue.text)) { - inputFieldValue = TextFieldValue("") + if (!imagePaste.isReceiving && viewModel.sendMessage(inputState.text.toString())) { + inputState.clearText() // Jump only after an accepted send. A readiness race keeps the draft intact. scrollController.jumpToBottom(animated = true) } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchController.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchController.kt index 8ed80979..81922aa3 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchController.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchController.kt @@ -1,5 +1,6 @@ package com.m57.hermescontrol.ui.chat +import com.m57.hermescontrol.ui.chat.fullbleed.isHiddenTool import com.m57.hermescontrol.ui.chat.fullbleed.isSyntheticSystemRow import com.m57.hermescontrol.ui.chat.fullbleed.isTimelineMarker import com.m57.hermescontrol.ui.chat.tool.ToolViewCache @@ -100,7 +101,7 @@ class ChatSearchController { } MessageRole.TOOL -> { - scan(idx, searchableToolText(message), SearchTarget.TOOL) + if (!message.isHiddenTool()) scan(idx, searchableToolText(message), SearchTarget.TOOL) } MessageRole.SYSTEM -> { diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSendStore.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSendStore.kt index c9ec1b01..ba15092c 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSendStore.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSendStore.kt @@ -61,6 +61,25 @@ internal fun pendingSendIdsConfirmedByDurableAliases( .toSet() } +/** + * #1427: the `prompt.submit` `user_row_id` is the exact gateway row for that send, so a history + * page containing it proves delivery even when the merge could not alias the local bubble. + */ +internal fun pendingSendIdsConfirmedByRowIds( + pageRowIds: Set, + pending: List, +): Set = + pending + .asSequence() + .filter { + it.state in setOf(PendingSendState.SENDING, PendingSendState.ACCEPTED, PendingSendState.UNKNOWN) + }.filter { it.userRowId != null && it.userRowId in pageRowIds } + .map { it.id } + .toSet() + +/** #1427: a receipt holding a gateway `user_row_id` is stored server-side and must never become UNKNOWN. */ +internal fun canDemoteAcceptedReceipt(receipt: PendingSend): Boolean = receipt.userRowId == null + /** Synchronous writes keep the queue recoverable when Android kills the process just after a tap. */ class ChatSendStore( private val prefs: SharedPreferences? = null, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt index af73df3f..33d7936c 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt @@ -232,7 +232,9 @@ internal fun mapServerMessages( rawContent } var attachments: List? = null - if (role == MessageRole.ASSISTANT && rawContent.contains("MEDIA:")) { + if (role == MessageRole.USER) { + attachments = userImageAttachments(finalContent, mediaUrl).takeIf { it.isNotEmpty() } + } else if (role == MessageRole.ASSISTANT && rawContent.contains("MEDIA:")) { val items = HostMediaExtractor.extract(rawContent) if (items.isNotEmpty()) { finalContent = HostMediaExtractor.strip(rawContent) @@ -275,7 +277,7 @@ internal fun mapServerMessages( completionId = completionId, serverRowId = msg.id?.toLong()?.takeIf { it > 0L }, reactions = msg.reactions, - ), + ).normalizedGatewayNotice(), ) } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt index c6b38fbd..5b7c7979 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt @@ -417,6 +417,12 @@ internal fun mergeCachedTranscriptPage( match.id to rich.copy( id = match.id, + attachments = + if (rich.role == MessageRole.USER) { + rich.attachments?.takeIf { it.isNotEmpty() } ?: message.attachments + } else { + rich.attachments + }, content = preservedContent ?: rich.content, restId = match.canonicalRestId ?: message.canonicalRestId, serverRowId = match.serverRowId ?: message.serverRowId, @@ -457,6 +463,15 @@ internal fun mergeTranscriptWithLive( when { match?.role == MessageRole.USER -> { match.copy( + // #1432: cached user rows lack attachment metadata; hydrate from REST, + // while preserving richer optimistic/local attachments when present. + attachments = match.attachments?.takeIf { it.isNotEmpty() } ?: message.attachments, + content = + if (match.isHistoricalCache && !message.attachments.isNullOrEmpty()) { + message.content + } else { + match.content + }, restId = (message.canonicalRestId ?: match.canonicalRestId).takeUnless { it == match.id }, serverRowId = message.serverRowId ?: match.serverRowId, reactions = message.reactions.ifEmpty { match.reactions }, @@ -531,14 +546,21 @@ internal fun mergeTranscriptWithLive( .mapNotNull { index -> matches[index]?.id?.let { id -> incoming[index].canonicalOrder?.let { id to it } } }.toMap() - return dedupeCachedMessages(transcript.inTranscriptOrder(current, resolvedOrders), confirmedOnly = true) - .reconcileReasoningRows() + return dedupeCachedMessages( + transcript.inTranscriptOrder( + previous = current, + resolvedOrders = resolvedOrders, + observedSuccessorAnchors = current, + ), + confirmedOnly = true, + ).reconcileReasoningRows() } /** Keep server order; place local notices and commands after their last preceding confirmed message, not at the transcript tail. */ private fun List.inTranscriptOrder( previous: List, resolvedOrders: Map, + observedSuccessorAnchors: List? = null, ): List { val latestCanonical = mapNotNull { it.canonicalOrder }.maxOrNull() ?: -1L // Session-start markers use -1 and must remain before unresolved cached history. @@ -549,6 +571,21 @@ private fun List.inTranscriptOrder( var pendingLocalOrder: Long? = null val localAnchors = mutableMapOf() val pendingOrderByLocal = mutableMapOf() + // #1451: an observed live block must stay before its later confirmed USER successor, + // even when its own REST echoes are absent. Successor anchors derive only from genuinely + // observed current lists (never cache concatenations) and only from later USER prompts. + val nextCanonicalOrder = mutableMapOf() + val anchorSource = observedSuccessorAnchors ?: emptyList() + var followingUserCanonical: Long? = null + for (i in anchorSource.indices.reversed()) { + val message = anchorSource[i] + val order = resolvedOrders[message.id] ?: message.canonicalOrder + if (order != null && message.role == MessageRole.USER) { + followingUserCanonical = order + } else if (followingUserCanonical != null) { + nextCanonicalOrder[message.id] = followingUserCanonical + } + } previous.forEach { message -> val order = resolvedOrders[message.id] ?: message.canonicalOrder if (order != null) { @@ -563,7 +600,7 @@ private fun List.inTranscriptOrder( } else if (message.isPermanentlyLocal()) { localAnchors[message.id] = if (hasPendingPredecessor) { - Long.MAX_VALUE + nextCanonicalOrder[message.id]?.let { it - 1L } ?: Long.MAX_VALUE } else { precedingCanonical?.takeIf { it >= 0L } ?: if (message.role == MessageRole.USER) { @@ -581,6 +618,9 @@ private fun List.inTranscriptOrder( } else { hasPendingPredecessor = true pendingLocalOrder = message.localOrder + nextCanonicalOrder[message.id]?.let { successor -> + localAnchors[message.id] = successor - 1L + } } } val previousIndices = previous.withIndex().associate { it.value.id to it.index } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt index 46663be7..7824fca0 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt @@ -41,6 +41,7 @@ import com.m57.hermescontrol.data.ws.contract.CommandDispatchParams import com.m57.hermescontrol.data.ws.contract.CommandsCatalogParams import com.m57.hermescontrol.data.ws.contract.ConfigGetParams import com.m57.hermescontrol.data.ws.contract.ConfigSetParams +import com.m57.hermescontrol.data.ws.contract.DESKTOP_SESSION_SOURCE import com.m57.hermescontrol.data.ws.contract.FileAttachParams import com.m57.hermescontrol.data.ws.contract.ImageAttachBytesParams import com.m57.hermescontrol.data.ws.contract.ProcessStopParams @@ -1511,6 +1512,10 @@ class ChatViewModel( viewModelScope.launch { fetchContextUsage() } } + is ReducerEffect.DeleteLocalMessage -> { + viewModelScope.launch(ioDispatcher) { repo.deleteMessage(effect.messageId) } + } + is ReducerEffect.AttachHostMedia -> { // Issue #724: turn host-path MEDIA: directives into real // attachments (images inline, every other file tappable) @@ -1804,6 +1809,8 @@ class ChatViewModel( contextBreakdown = null, compressionCount = null, sessionUsage = null, + isCompressing = false, + compressionStatus = null, ) } publishPendingSends() @@ -3676,6 +3683,34 @@ class ChatViewModel( fun addAttachments(attachments: List) = attachmentsDelegate.addAttachments(attachments) + /** Clipboard reads are asynchronous: bind them to this exact session generation and server. */ + internal fun captureAttachmentTarget(): ChatAttachmentTarget? { + val state = _uiState.value + val sessionId = state.currentSessionId ?: return null + // Connection status is combined into public uiState, not written to _uiState. + if (wsClient.connectionStatus.value != ConnectionStatus.CONNECTED || + !state.isSessionReady || _timelineState.value.isHistorical + ) { + return null + } + return ChatAttachmentTarget( + sessionId = sessionId, + generation = sessionGeneration, + baseUrl = AuthManager.getBaseUrl(), + connectionProfileId = AuthManager.getSelectedProfileId(), + agentProfileId = AuthManager.activeProfileId.value, + ) + } + + internal fun addPastedAttachments( + target: ChatAttachmentTarget, + attachments: List, + ): Boolean { + if (captureAttachmentTarget() != target) return false + attachmentsDelegate.addAttachments(attachments) + return true + } + fun removeAttachment(index: Int) = attachmentsDelegate.removeAttachment(index) fun openAttachment(attachment: Attachment) = mediaDelegate.openAttachment(attachment) @@ -4127,44 +4162,46 @@ class ChatViewModel( val arg = parts.getOrElse(1) { "" } viewModelScope.launch(ioDispatcher) { try { - // Primary path: command.dispatch handles quick/plugin/bundle/ - // skill commands + a few hardcoded ones. It returns a hard 4018 - // "not a ... command" for everything that lives only in the TUI - // slash worker (the 29 commands that 4018'd on mobile — issue - // #576). For those we fall back to slash.exec, which runs the - // full COMMAND_REGISTRY through the worker. + // Desktop parity: slash.exec owns generic commands. Handle errors here, + // not again through the shared RpcError banner (/usage regression). val result = wsClient.call( - RpcMethods.COMMAND_DISPATCH, - CommandDispatchParams(name = name, arg = arg, sessionId = sessionId), + RpcMethods.SLASH_EXEC, + SlashExecParams(sessionId = sessionId, command = command.removePrefix("/")), + suppressErrorEvent = true, ) - handleDispatchResult(result.toAny()) - } catch (e: HermesWsClient.HermesRpcException) { - val msg = e.message.orEmpty() - // Registry miss on command.dispatch: the backend emits exactly - // "not a quick/plugin/bundle/skill command: " (tui_gateway - // server.py L12408). Match that precise phrase so unrelated - // errors can't accidentally trigger the slash.exec fallback. - if (msg.contains("not a quick/plugin/bundle/skill command")) { - // Registry miss on command.dispatch -> retry via slash.exec, - // which routes the full CLI command set through the worker. - try { - val result = - wsClient.call( - RpcMethods.SLASH_EXEC, - SlashExecParams( - sessionId = sessionId, - command = "/$name${if (arg.isNotEmpty()) " $arg" else ""}", - ), - ) - val output = (result.toAny() as? Map<*, *>)?.get("output") as? String - if (!output.isNullOrBlank()) addAssistantMessage(output) - } catch (e2: HermesWsClient.HermesRpcException) { - addAssistantMessage("/$name: ${e2.message}") - } + val map = result.toAny() as? Map<*, *> + if (map?.get("type") is String) { + handleDispatchResult(map) } else { - // Legit error from command.dispatch (busy, no history, etc.) - addAssistantMessage("/$name: ${e.message}") + val output = (map?.get("output") as? String).orEmpty().ifBlank { "/$name: no output" } + val warning = (map?.get("warning") as? String).orEmpty() + addAssistantMessage(if (warning.isBlank()) output else "warning: $warning\n$output") + } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + try { + val result = + wsClient.call( + RpcMethods.COMMAND_DISPATCH, + CommandDispatchParams(name = name, arg = arg, sessionId = sessionId), + suppressErrorEvent = true, + ) + val map = result.toAny() as? Map<*, *> + if (map?.get("type") is String) { + handleDispatchResult(map) + } else { + addAssistantMessage("/$name: invalid response: command.dispatch") + } + } catch (fallback: CancellationException) { + throw fallback + } catch (fallback: Exception) { + val registryMiss = + Regex("not a quick/plugin/(?:bundle/)?skill command", RegexOption.IGNORE_CASE) + .containsMatchIn(fallback.message.orEmpty()) + val failure = if (registryMiss) e else fallback + addAssistantMessage("/$name: ${failure.message}") } } } @@ -4534,7 +4571,7 @@ class ChatViewModel( viewModelScope.launch(ioDispatcher) { wsClient.send( RpcMethods.SESSION_CREATE, - SessionCreateParams(source = "desktop"), + SessionCreateParams(source = DESKTOP_SESSION_SOURCE), onSent = { id -> trackSessionRequest(id, WsMethods.SESSION_CREATE, generation) }, ) } @@ -5070,7 +5107,22 @@ class ChatViewModel( val page = withContext(historyDispatcher) { repo.loadPage(sessionId, before, MESSAGE_PAGE_SIZE) } val valid = { isCurrent() && cacheCursor == before } if (!valid()) return false - mergeHistoryPage(valid, cached = true) { page.messages } ?: return false + mergeHistoryPage(valid, cached = true) { + // #1432: Room retains host references, not connection-bound download URLs. + page.messages.map { message -> + if (message.role == MessageRole.USER && message.attachments.isNullOrEmpty()) { + message.copy( + attachments = + userImageAttachments( + message.content, + ::gatewayMediaUrl, + ).takeIf { it.isNotEmpty() }, + ) + } else { + message + } + } + } ?: return false if (!valid()) return false cacheCursor = page.cursor cacheHasOlder = page.hasOlder @@ -5216,7 +5268,11 @@ class ChatViewModel( val scope = sendScope() val scopedPending = sendStore.all().filter { it.scope == scope && it.sessionId == sessionId } - pendingSendIdsConfirmedByDurableAliases(aliases, scopedPending).forEach(::removePendingSend) + val pageRowIds = page.mapNotNull { it.serverRowId }.toSet() + ( + pendingSendIdsConfirmedByDurableAliases(aliases, scopedPending) + + pendingSendIdsConfirmedByRowIds(pageRowIds, scopedPending) + ).forEach(::removePendingSend) publishPendingSends() drainPendingQueue() } @@ -5324,6 +5380,9 @@ class ChatViewModel( contextBreakdown = null, compressionCount = null, sessionUsage = null, + // #1433: compaction state belongs to the session being left. + isCompressing = false, + compressionStatus = null, pendingAttachments = emptyList(), composerTextToRestore = null, reactionKind = null, @@ -5352,6 +5411,7 @@ class ChatViewModel( val params = SessionResumeParams( sessionId = sessionId, + source = DESKTOP_SESSION_SOURCE, omitMessages = true, profile = profile?.takeIf { it.isNotBlank() }, ) @@ -5709,7 +5769,8 @@ class ChatViewModel( persistHistoryPage(page, sessionId) if (valid() && !mainTurnBusy && mainTurnEpoch == completedTurnEpoch) { unverifiedAccepted.forEach { receipt -> - if (sendStore.all().any { it == receipt }) { + // A gateway-issued user_row_id means the row is stored: never walk back to UNKNOWN. + if (canDemoteAcceptedReceipt(receipt) && sendStore.all().any { it == receipt }) { markPendingSend(receipt.id, PendingSendState.UNKNOWN) } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt index 65ecedfe..fa3fc094 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt @@ -136,6 +136,7 @@ object ChatWsEventReducer { is WsEvent.VaultSaveLoginExpire -> event.sessionId is WsEvent.VaultCodeRequest -> event.sessionId is WsEvent.VaultCodeExpire -> event.sessionId + is WsEvent.StatusUpdate -> event.sessionId else -> null } if (eventSessionId != null && (currentSessionId == null || eventSessionId != currentSessionId)) { @@ -457,6 +458,7 @@ object ChatWsEventReducer { ): ReducerResult { val failure = replyFailureFromPayload(event.rawPayload, event.text) if (failure != null) return onReplyFailure(state, streamingState, event, failure) + if (GatewayNotice.isInterruptSentinel(event.text)) return onInterruptNotice(state, streamingState, event) val finalSnapshot = event.rawPayload?.let(::parseUsageSnapshot) val usageState = finalSnapshot?.let { applyUsageSnapshot(state, it) } ?: state val turnUsage = finalSnapshot?.deltaFrom(streamingState.turnUsageBaseline) @@ -1052,36 +1054,107 @@ object ChatWsEventReducer { event: WsEvent.StatusUpdate, ): ReducerResult { val kind = event.data?.get("kind") as? String - val text = event.data?.get("text") as? String + val text = (event.data?.get("text") as? String)?.trim() return when (kind) { + // In-progress compaction is live state: the tail indicator shows only while it runs. "compressing", "compacting" -> { ReducerResult( state = state.copy( isCompressing = true, - compressionStatus = text ?: "⏳ Compressing context...", + compressionStatus = text?.ifBlank { null } ?: "⏳ Compressing context...", ), streamingState = streamingState, ) } + // #1433: the done edge is a transcript event, seated where it happened, never a floating tail row. "compacted" -> { + appendGatewayNotice(state.copy(isCompressing = false, compressionStatus = null), streamingState, text) + } + + "ready" -> { ReducerResult( - state = - state.copy( - isCompressing = false, - compressionStatus = text, - ), + state = state.copy(isCompressing = false, compressionStatus = null), streamingState = streamingState, ) } + // Durable notices the desktop/TUI also paint into the transcript. + "goal", "loop", "warn", "fallback" -> { + appendGatewayNotice(state, streamingState, text) + } + else -> { ReducerResult(state = state, streamingState = streamingState) } } } + /** + * Appends a gateway notice as a persisted SYSTEM row (#1433). SYSTEM rows are permanently local, so + * `inTranscriptOrder` anchors them after their preceding confirmed row instead of the transcript tail. + * A replayed event (reconnect `session.events.since`) re-sends the same text within the current turn; + * skip it rather than stacking a duplicate. + */ + private fun appendGatewayNotice( + state: ChatUiState, + streamingState: StreamingState, + text: String?, + extraEffects: List = emptyList(), + ): ReducerResult { + if (text.isNullOrBlank()) { + return ReducerResult(state = state, streamingState = streamingState, effects = extraEffects) + } + val turnStart = state.messages.indexOfLast { it.isUserTurnBoundary() } + 1 + val duplicate = + state.messages.drop(turnStart).any { it.role == MessageRole.SYSTEM && it.content == text } + if (duplicate) return ReducerResult(state = state, streamingState = streamingState, effects = extraEffects) + val notice = ChatMessage(role = MessageRole.SYSTEM, content = text) + val sid = state.currentSessionId + return ReducerResult( + state = state.copy(messages = state.messages + notice), + streamingState = streamingState, + effects = extraEffects + listOfNotNull(sid?.let { ReducerEffect.PersistMessage(notice, it) }), + ) + } + + /** + * A turn whose final text is a gateway interrupt sentinel ends with a notice, not assistant prose (#1433). + * For the repetition-loop sentinel the gateway hides the looped partial; drop it here too, including a + * copy already sealed by Stop, or it lingers as a live-only row that jumps to the tail on every merge. + */ + private fun onInterruptNotice( + state: ChatUiState, + streamingState: StreamingState, + event: WsEvent.MessageComplete, + ): ReducerResult { + val usageState = event.rawPayload?.let(::parseUsageSnapshot)?.let { applyUsageSnapshot(state, it) } ?: state + val text = event.text.trim() + val effects = mutableListOf() + var messages = usageState.messages + if (text == GatewayNotice.REPETITION_LOOP_INTERRUPTED) { + val looped = setOfNotNull(streamingState.interruptedMessage?.id, streamingState.streamingMessage?.id) + if (looped.isNotEmpty()) { + messages = messages.filterNot { it.id in looped } + looped.forEach { effects += ReducerEffect.DeleteLocalMessage(it) } + } + } else { + // Other exits keep any real partial the user already saw. + streamingState.streamingMessage + ?.takeIf { it.content.isNotBlank() || it.reasoningText.isNotBlank() } + ?.copy(isStreaming = false, finishTimestamp = System.currentTimeMillis()) + ?.let { partial -> + messages = messages.upsertById(partial) + usageState.currentSessionId?.let { effects += ReducerEffect.PersistMessage(partial, it) } + } + } + effects += ReducerEffect.RefreshSessions + effects += ReducerEffect.RefreshContextUsage + val settled = usageState.copy(messages = messages, isAgentTyping = false, clarifyRequest = null) + return appendGatewayNotice(settled, StreamingState(), text, effects) + } + /** Paints a live tapback on the row whose gateway id matches; unmatched rows arrive with the next REST load. */ private fun onMessageReaction( state: ChatUiState, @@ -1293,6 +1366,11 @@ sealed class ReducerEffect { data object RefreshContextUsage : ReducerEffect() + /** Drop a never-confirmed local row from Room (the DAO refuses rows that carry a REST identity). */ + data class DeleteLocalMessage( + val messageId: String, + ) : ReducerEffect() + /** * Ask the ViewModel to turn any `MEDIA:` directives in the * just-finalized assistant message into real [Attachment]s (images inline, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/GatewayNotice.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/GatewayNotice.kt new file mode 100644 index 00000000..a61d3e4e --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/GatewayNotice.kt @@ -0,0 +1,32 @@ +package com.m57.hermescontrol.ui.chat + +/** + * Gateway turn-outcome sentinels that arrive as assistant text but are notices, not prose (#1433). + * + * Sources in hermes-agent: `agent/repetition_guard.py` (`REPETITION_LOOP_INTERRUPTED`) and the + * `interrupt_text` / `close_interrupted_tool_sequence` exits ("Operation interrupted…"). They have no + * matching visible REST row (or a model-facing closing row), so as assistant prose they never get a + * canonical position and jump to the transcript tail on every merge. They render as system rows instead. + */ +internal object GatewayNotice { + const val REPETITION_LOOP_INTERRUPTED = "[the reply degenerated into a repetition loop and was interrupted]" + + // "Operation interrupted." / "Operation interrupted: ." / "Operation interrupted during retry (…)." + private val OPERATION_INTERRUPTED = Regex("""^Operation interrupted(?:\.|(?::| during) [^\n]*\.)$""") + + fun isInterruptSentinel(text: String): Boolean { + val trimmed = text.trim() + return trimmed == REPETITION_LOOP_INTERRUPTED || OPERATION_INTERRUPTED.matches(trimmed) + } +} + +/** + * The gateway persists an interrupted turn's closing row as `role=assistant` with the sentinel text. Map it + * (REST and Room alike) to the same SYSTEM notice the live path appends, so every copy matches one row. + */ +internal fun ChatMessage.normalizedGatewayNotice(): ChatMessage = + if (role == MessageRole.ASSISTANT && attachments.isNullOrEmpty() && GatewayNotice.isInterruptSentinel(content)) { + copy(role = MessageRole.SYSTEM, content = content.trim(), reasoningText = "", completionId = null) + } else { + this + } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcher.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcher.kt index 65221a8a..9c552f7f 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcher.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcher.kt @@ -106,7 +106,7 @@ sealed class SlashResult { /** Create a new session (client-side immediate). */ data object NewSession : SlashResult() - /** Forward to command.dispatch via WebSocket. */ + /** Execute through slash.exec, with command.dispatch as the desktop-compatible fallback. */ data object RpcDispatch : SlashResult() /** Fork the active conversation via the session.branch WebSocket RPC. */ diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/UserBubble.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/UserBubble.kt index 375726f0..4b5377df 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/UserBubble.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/UserBubble.kt @@ -25,6 +25,8 @@ import androidx.compose.foundation.text.selection.SelectionContainer import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Check import androidx.compose.material.icons.filled.ContentCopy +import androidx.compose.material.icons.filled.Done +import androidx.compose.material.icons.filled.DoneAll import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme @@ -58,6 +60,7 @@ import com.m57.hermescontrol.theme.DarkOnSurface import com.m57.hermescontrol.theme.LightOnSurface import com.m57.hermescontrol.theme.LocalHermesStatusColors import com.m57.hermescontrol.ui.chat.components.rememberCopyFeedback +import com.m57.hermescontrol.ui.chat.tool.ToolJson import com.m57.hermescontrol.util.BidiUtils import kotlinx.coroutines.launch @@ -100,18 +103,36 @@ fun UserBubble( var copied by rememberCopyFeedback() val statusColors = LocalHermesStatusColors.current + val deliveryState = + when { + pendingSendState == PendingSendState.SENDING -> DeliveryState.SENT + pendingSendState == PendingSendState.ACCEPTED || + message.canonicalRestId != null || message.serverRowId != null -> DeliveryState.DELIVERED + + else -> null + } + + // #1432: also protect cached/legacy rows and unexpectedly large plain-string payloads. + // Keep the original content for Copy; only the layout input is bounded. + val displayContent = + remember(message.content, message.attachments) { + // #1432: the path is only hidden when its image renders as an attachment below. + val visible = + if (message.attachments.isNullOrEmpty()) message.content else hideImageRefLines(message.content) + ToolJson.clampForDisplay(visible) + } val highlightedText = - remember(message.content, searchQuery, isCurrentMatch, statusColors) { + remember(displayContent, searchQuery, isCurrentMatch, statusColors) { if (searchQuery.isNotBlank()) { buildHighlightedString( - message.content, + displayContent, searchQuery, isCurrentMatch, statusColors, ) } else { - AnnotatedString(message.content) + AnnotatedString(displayContent) } } Box( @@ -153,7 +174,7 @@ fun UserBubble( color = Color.Transparent, tonalElevation = 0.dp, ) { - val isRtl = remember(message.content) { BidiUtils.isRtlText(message.content) } + val isRtl = remember(displayContent) { BidiUtils.isRtlText(displayContent) } val bubbleDirection = if (isRtl) LayoutDirection.Rtl else LocalLayoutDirection.current CompositionLocalProvider(LocalLayoutDirection provides bubbleDirection) { Column(modifier = Modifier.padding(horizontal = 10.dp, vertical = 6.dp)) { @@ -183,23 +204,6 @@ fun UserBubble( canSave = canSaveAttachment, onImageClick = onImageClick, ) - // #1427: keep the live text and media visible with an honest delivery label. - if (pendingSendState == PendingSendState.SENDING || - pendingSendState == PendingSendState.ACCEPTED - ) { - Text( - stringResource( - if (pendingSendState == PendingSendState.SENDING) { - R.string.chat_pending_sending - } else { - R.string.chat_send_accepted - }, - ), - color = userBubbleTextColor, - style = MaterialTheme.typography.labelSmall, - modifier = Modifier.testTag("user_send_status_${message.id}"), - ) - } if (!message.isStreaming) { FlowRow( modifier = @@ -236,6 +240,11 @@ fun UserBubble( color = userBubbleTextColor.copy(alpha = 0.6f), style = MaterialTheme.typography.labelSmall, ) + DeliveryChecks( + state = deliveryState, + tint = userBubbleTextColor, + messageId = message.id, + ) if (messageStatsEnabled && showUserMessageTokens && message.tokenCount != null && message.tokenCount > 0 ) { @@ -268,3 +277,24 @@ fun UserBubble( } } } + +/** WhatsApp-style delivery ticks: one dim check while sending, two once the server has the prompt. */ +private enum class DeliveryState { SENT, DELIVERED } + +@Composable +private fun DeliveryChecks( + state: DeliveryState?, + tint: Color, + messageId: String, +) { + if (state == null) return + val delivered = state == DeliveryState.DELIVERED + Spacer(modifier = Modifier.width(4.dp)) + Icon( + imageVector = if (delivered) Icons.Filled.DoneAll else Icons.Filled.Done, + contentDescription = + stringResource(if (delivered) R.string.chat_send_accepted else R.string.chat_pending_sending), + modifier = Modifier.size(14.dp).testTag("user_send_status_$messageId"), + tint = tint.copy(alpha = if (delivered) 1f else 0.6f), + ) +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/UserImageAttachments.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/UserImageAttachments.kt new file mode 100644 index 00000000..4e09bb15 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/UserImageAttachments.kt @@ -0,0 +1,63 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.model.Attachment +import com.m57.hermescontrol.data.model.AttachmentSource + +/** + * Parses user-authored or server-persisted `@image:` directives from message content into + * gateway-backed [Attachment]s. + * + * Requirements: + * - Recognizes whole trimmed lines `@image:/absolute/path` (spaces in path allowed). + * - Ignores inline prose refs (e.g. "look at @image:/path in the middle of a sentence"). + * - Ignores empty or relative paths (must start with '/'). + * - Preserves unique paths across lines (deduplicates repeated references to the same path). + * - Resolves each path via [mediaUrl]; drops attachments where [mediaUrl] returns null. + * - Constructs [Attachment]s with source = [AttachmentSource.GATEWAY], resolved uri/gatewayUrl, + * size = 0, name = [mediaNameFromPath], mimeType = [mediaMimeForPath]. + */ +internal fun userImageAttachments( + content: String, + mediaUrl: (String) -> String?, +): List { + if (!content.contains("@image:")) return emptyList() + + val seenPaths = mutableSetOf() + val attachments = mutableListOf() + + for (rawLine in content.lines()) { + val line = rawLine.trim() + if (!line.startsWith("@image:")) continue + val path = line.removePrefix("@image:").trim() + if (!path.startsWith("/")) continue + if (!seenPaths.add(path)) continue + + val url = mediaUrl(path) ?: continue + attachments.add( + Attachment( + uri = url, + name = mediaNameFromPath(path), + mimeType = mediaMimeForPath(path), + size = 0, + gatewayUrl = url, + source = AttachmentSource.GATEWAY, + ), + ) + } + + return attachments +} + +/** Hides whole-line `@image:/abs/path` refs from bubble text once they render as attachments. */ +internal fun hideImageRefLines(content: String): String { + if (!content.contains("@image:")) return content + return content + .lines() + .filterNot { line -> + line.trim().let { + it.startsWith("@image:") && + it.removePrefix("@image:").trim().startsWith("/") + } + }.joinToString("\n") + .trim() +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatComposer.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatComposer.kt index 16a91ddf..60ebd62b 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatComposer.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatComposer.kt @@ -10,8 +10,11 @@ import androidx.compose.animation.shrinkVertically import androidx.compose.animation.slideInVertically import androidx.compose.animation.slideOutVertically import androidx.compose.foundation.BorderStroke +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background import androidx.compose.foundation.clickable +import androidx.compose.foundation.content.ReceiveContentListener +import androidx.compose.foundation.content.contentReceiver import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -32,6 +35,9 @@ import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.text.BasicTextField +import androidx.compose.foundation.text.input.InputTransformation +import androidx.compose.foundation.text.input.TextFieldLineLimits +import androidx.compose.foundation.text.input.TextFieldState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.InsertDriveFile import androidx.compose.material.icons.filled.Close @@ -39,6 +45,7 @@ import androidx.compose.material.icons.filled.Lock import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.Icon import androidx.compose.material3.IconButton +import androidx.compose.material3.LinearProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Surface import androidx.compose.material3.Text @@ -64,8 +71,9 @@ import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.platform.LocalSoftwareKeyboardController import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextDirection import androidx.compose.ui.text.style.TextOverflow @@ -86,9 +94,9 @@ import com.m57.hermescontrol.util.BidiUtils * controls row (attach, model/reasoning pill, mic, send) inside it below. */ @Composable +@OptIn(ExperimentalFoundationApi::class) fun ChatInputBar( - inputFieldValue: TextFieldValue, - onInputChange: (TextFieldValue) -> Unit, + inputState: TextFieldState, onSend: () -> Unit, onMicTap: () -> Unit, isListening: Boolean, @@ -127,15 +135,18 @@ fun ChatInputBar( isRecordingVoice: Boolean = false, amplitudeProvider: () -> Float = { 0f }, onStopGeneration: () -> Unit = {}, + receiveContentListener: ReceiveContentListener? = null, + isReceivingContent: Boolean = false, ) { + val inputText = inputState.text.toString() // Allow sending while the agent is mid-turn or awaiting approval: the // gateway's prompt.submit busy-input policy queues it as the next turn // (tui_gateway/server.py:_handle_busy_submit), so the message is never // dropped. Slash commands were already allowed; regular prompts now are too. val canSend = - pendingReasoningLevel == null && - ChatInputPolicy.canSend(inputFieldValue.text, pendingAttachments, isConnected, isSessionReady) - val hasDraft = inputFieldValue.text.isNotBlank() || pendingAttachments.isNotEmpty() + !isReceivingContent && pendingReasoningLevel == null && + ChatInputPolicy.canSend(inputText, pendingAttachments, isConnected, isSessionReady) + val hasDraft = inputText.isNotBlank() || pendingAttachments.isNotEmpty() // Attachment tray state var showAttachmentTray by remember { mutableStateOf(false) } @@ -232,6 +243,16 @@ fun ChatInputBar( border = BorderStroke(width = 1.dp, color = palette.cardBorder), ) { Column(modifier = Modifier.padding(top = 8.dp, bottom = 10.dp)) { + if (isReceivingContent) { + val loadingLabel = stringResource(R.string.chat_image_paste_loading) + LinearProgressIndicator( + modifier = + Modifier + .fillMaxWidth() + .testTag("chat_image_paste_progress") + .semantics { contentDescription = loadingLabel }, + ) + } // Commands hidden from the suggestion menu — desktop/CLI-only and // TUI-only commands that don't function on mobile (issue #574). // Single source of truth: CommandBlocklist.UNSUPPORTED, which is @@ -245,13 +266,13 @@ fun ChatInputBar( .filter { it.lowercase() !in hiddenSlashDisplay } androidx.compose.animation.AnimatedVisibility( - visible = inputFieldValue.text.startsWith("/") && !inputFieldValue.text.contains(" "), + visible = inputText.startsWith("/") && !inputText.contains(" "), enter = androidx.compose.animation.fadeIn() + androidx.compose.animation.expandVertically(), exit = androidx.compose.animation.fadeOut() + androidx.compose.animation.shrinkVertically(), ) { val filteredCommands = ChatInputPolicy.sortSlashSuggestions( - commandNames.filter { it.startsWith(inputFieldValue.text, ignoreCase = true) }, + commandNames.filter { it.startsWith(inputText, ignoreCase = true) }, slashUsageCounts, ) if (filteredCommands.isNotEmpty()) { @@ -280,7 +301,7 @@ fun ChatInputBar( color = MaterialTheme.colorScheme.primary, ) }, - onClick = { onInputChange(ChatInputPolicy.commandFieldValue(cmd)) }, + onClick = { inputState.replaceComposerDraft(cmd) }, ) } } @@ -341,7 +362,7 @@ fun ChatInputBar( stringResource(R.string.chat_input_placeholder_not_connected) } - ChatInputPolicy.showQueuePlaceholder(inputFieldValue.text, isAgentTyping) -> { + ChatInputPolicy.showQueuePlaceholder(inputText, isAgentTyping) -> { stringResource(R.string.chat_input_placeholder_queue) } @@ -356,9 +377,9 @@ fun ChatInputBar( val ambientLayoutDirection = LocalLayoutDirection.current val inputLayoutDirection = - remember(inputFieldValue.text, ambientLayoutDirection) { + remember(inputText, ambientLayoutDirection) { BidiUtils.resolveLayoutDirection( - inputFieldValue.text, + inputText, fallback = ambientLayoutDirection, ) } @@ -381,8 +402,7 @@ fun ChatInputBar( .heightIn(min = 42.dp), ) { BasicTextField( - value = inputFieldValue, - onValueChange = { if (!isRecordingVoice) onInputChange(it) }, + state = inputState, modifier = Modifier .fillMaxWidth() @@ -392,18 +412,28 @@ fun ChatInputBar( .focusRequester(inputFocusRequester) .onFocusChanged { focusState -> inputFocused = focusState.isFocused - }.testTag("chat_input"), + }.testTag("chat_input") + .then( + if (receiveContentListener != null) { + Modifier.contentReceiver(receiveContentListener) + } else { + Modifier + }, + ), enabled = isConnected, + inputTransformation = + InputTransformation { + if (isRecordingVoice) revertAllChanges() + }, textStyle = MaterialTheme.typography.bodyLarge.copy( color = palette.text, textAlign = if (isInputRtl) TextAlign.Right else TextAlign.Left, textDirection = if (isInputRtl) TextDirection.Rtl else TextDirection.Ltr, ), - singleLine = false, - maxLines = 8, + lineLimits = TextFieldLineLimits.MultiLine(maxHeightInLines = 8), cursorBrush = SolidColor(palette.text), - decorationBox = { innerTextField -> + decorator = { innerTextField -> Box( modifier = Modifier.fillMaxWidth(), contentAlignment = @@ -416,7 +446,7 @@ fun ChatInputBar( CompositionLocalProvider( LocalLayoutDirection provides inputLayoutDirection, ) { - if (inputFieldValue.text.isEmpty() && !isRecordingVoice) { + if (inputText.isEmpty() && !isRecordingVoice) { Text( text = placeholderText, style = MaterialTheme.typography.bodyLarge, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatComposerState.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatComposerState.kt new file mode 100644 index 00000000..f75e9084 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatComposerState.kt @@ -0,0 +1,31 @@ +package com.m57.hermescontrol.ui.chat.components + +import androidx.compose.foundation.text.input.TextFieldState +import androidx.compose.ui.text.input.TextFieldValue +import com.m57.hermescontrol.ui.chat.ChatInputPolicy + +/** Apply an intentional external draft replacement, never a value mirror during ordinary typing. */ +internal fun TextFieldState.replaceComposerDraft(text: String) { + val replacement = ChatInputPolicy.commandFieldValue(text) + edit { + replace(0, length, replacement.text) + selection = replacement.selection + } +} + +/** Recheck the live draft when a rejected send is returned; it may have changed since dispatch. */ +internal fun TextFieldState.restoreRejectedComposerDraft(rejectedText: String) { + val current = TextFieldValue(text.toString(), selection) + val restored = ChatInputPolicy.restoreRejectedText(rejectedText, current) + edit { + replace(0, length, restored.text) + selection = restored.selection + } +} + +/** Speech results arrive asynchronously: append to the draft as it exists at result time. */ +internal fun TextFieldState.appendSpeechComposerDraft(spokenText: String) { + if (spokenText.isBlank()) return + val current = text.toString() + replaceComposerDraft(if (current.isBlank()) spokenText else "$current $spokenText") +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatImagePasteController.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatImagePasteController.kt new file mode 100644 index 00000000..375d9323 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatImagePasteController.kt @@ -0,0 +1,195 @@ +package com.m57.hermescontrol.ui.chat.components + +import android.net.Uri +import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.content.MediaType +import androidx.compose.foundation.content.ReceiveContentListener +import androidx.compose.foundation.content.TransferableContent +import androidx.compose.foundation.content.consume +import androidx.compose.foundation.content.hasMediaType +import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.setValue +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.LocalResources +import com.m57.hermescontrol.R +import com.m57.hermescontrol.data.model.Attachment +import com.m57.hermescontrol.ui.chat.ChatAttachmentTarget +import com.m57.hermescontrol.ui.chat.ChatPastedImageStore +import com.m57.hermescontrol.ui.chat.ChatViewModel +import com.m57.hermescontrol.ui.chat.MAX_CHAT_ATTACHMENT_BYTES +import com.m57.hermescontrol.ui.chat.PastedImage +import com.m57.hermescontrol.ui.chat.PastedImageTooLargeException +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import java.io.IOException + +internal const val MAX_PASTED_IMAGE_COUNT = 8 +internal const val MAX_PASTED_IMAGE_BATCH_BYTES = 20L * 1024L * 1024L + +private class PastedImageBatchLimitException : IOException("Pasted image batch exceeds its limit") + +/** Receives explicit paste/IME commits only; never reads or observes the system clipboard. */ +@OptIn(ExperimentalFoundationApi::class) +internal class ChatImagePasteController( + private val scope: CoroutineScope, + private val importImage: suspend (Uri, Long) -> PastedImage?, + private val captureTarget: () -> ChatAttachmentTarget?, + private val commit: (ChatAttachmentTarget, List) -> Boolean, + private val onError: (Int) -> Unit, + private val isImageUri: (Uri) -> Boolean = { false }, +) : ReceiveContentListener { + var isReceiving by mutableStateOf(false) + private set + + private var closed = false + private var job: Job? = null + private val retainedPayloads = mutableSetOf() + + override fun onReceive(transferableContent: TransferableContent): TransferableContent? { + if (!transferableContent.hasMediaType(MediaType.Image)) return transferableContent + // Drag grants have a different lifecycle; this feature handles paste and IME commits only. + if (transferableContent.source != TransferableContent.Source.Clipboard && + transferableContent.source != TransferableContent.Source.Keyboard + ) { + return transferableContent + } + val clip = transferableContent.clipEntry.clipData + val uris = ArrayList(MAX_PASTED_IMAGE_COUNT + 1) + for (index in 0 until clip.itemCount) { + val uri = clip.getItemAt(index).uri + if (uri?.scheme == "content" && isImageUri(uri)) { + uris.add(uri) + if (uris.size > MAX_PASTED_IMAGE_COUNT) break + } + } + if (!receiveImages(uris, transferableContent)) return transferableContent + // Only accepted image candidates are consumed. Text and non-image URIs + // remain available to the ordinary text field/other receivers. + val accepted = uris.toSet() + // An item can carry both image URI and text. Keep that item for the text + // field after importing its image; consuming it would drop its text. + return transferableContent.consume { it.uri in accepted && it.text == null && it.htmlText == null } + } + + /** [payload] strongly retains Compose's temporary InputContentInfo permission until IO ends. */ + internal fun receiveImages( + uris: List, + payload: Any, + ): Boolean { + if (closed || !scope.isActive || uris.isEmpty()) return false + if (isReceiving) { + onError(R.string.chat_image_paste_busy) + return true + } + if (uris.size > MAX_PASTED_IMAGE_COUNT) { + onError(R.string.chat_image_paste_batch_limit) + return true + } + val target = captureTarget() + if (target == null) { + onError(R.string.chat_session_not_ready) + return true + } + isReceiving = true + retainedPayloads.add(payload) + job = + scope.launch { + val images = mutableListOf() + var remainingBytes = MAX_PASTED_IMAGE_BATCH_BYTES + var committed = false + try { + for (uri in uris) { + if (remainingBytes == 0L) throw PastedImageBatchLimitException() + val cap = minOf(remainingBytes, MAX_CHAT_ATTACHMENT_BYTES) + val image = importImage(uri, cap) ?: throw IOException("Unreadable pasted image") + images += image + remainingBytes -= image.file.length() + if (remainingBytes < 0L) throw PastedImageBatchLimitException() + } + currentCoroutineContext().ensureActive() + if (!closed && captureTarget() == target) { + committed = commit(target, images.map { it.attachment }) + } + } catch (cancelled: CancellationException) { + throw cancelled + } catch (failure: Exception) { + if (!closed && captureTarget() == target) { + onError( + if (failure is PastedImageBatchLimitException || + (failure is PastedImageTooLargeException && remainingBytes < MAX_CHAT_ATTACHMENT_BYTES) + ) { + R.string.chat_image_paste_batch_limit + } else if (failure is PastedImageTooLargeException) { + R.string.chat_image_paste_too_large + } else { + R.string.chat_image_paste_failed + }, + ) + } + } finally { + if (!committed) { + withContext(NonCancellable + Dispatchers.IO) { images.forEach { it.discard() } } + } + retainedPayloads.remove(payload) + isReceiving = false + } + } + // Covers a scope cancelled before the launched coroutine ever starts. + job?.invokeOnCompletion { retainedPayloads.remove(payload) } + return true + } + + fun close() { + closed = true + job?.cancel() + } +} + +@Composable +@OptIn(ExperimentalFoundationApi::class) +internal fun rememberChatImagePaste( + viewModel: ChatViewModel, + onShowMessage: (String) -> Unit, +): ChatImagePasteController { + val context = LocalContext.current + val resources by rememberUpdatedState(LocalResources.current) + val scope = rememberCoroutineScope() + val showMessage by rememberUpdatedState(onShowMessage) + val controller = + remember(viewModel, context, scope) { + val store = ChatPastedImageStore(context.applicationContext) + ChatImagePasteController( + scope = scope, + importImage = store::importImage, + isImageUri = { uri -> + try { + context.contentResolver.getType(uri)?.let { type -> + type.startsWith("image/", ignoreCase = true) && type.length > "image/".length + } == true + } catch (_: RuntimeException) { + // Provider failures must never consume an unclassified URI. + false + } + }, + captureTarget = viewModel::captureAttachmentTarget, + commit = viewModel::addPastedAttachments, + onError = { showMessage(resources.getString(it)) }, + ) + } + DisposableEffect(controller) { onDispose { controller.close() } } + return controller +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatMediaLaunchers.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatMediaLaunchers.kt index fb013b82..b90cb276 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatMediaLaunchers.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatMediaLaunchers.kt @@ -11,6 +11,7 @@ import android.speech.RecognizerIntent import android.util.Log import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts +import androidx.compose.foundation.text.input.TextFieldState import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect @@ -21,13 +22,11 @@ import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource -import androidx.compose.ui.text.input.TextFieldValue import androidx.core.content.ContextCompat import androidx.core.content.FileProvider import com.m57.hermescontrol.ExternalActivityLifecycleGuard import com.m57.hermescontrol.R import com.m57.hermescontrol.data.model.Attachment -import com.m57.hermescontrol.ui.chat.ChatInputPolicy import com.m57.hermescontrol.ui.chat.SpeechInputHelper import com.m57.hermescontrol.ui.chat.VoiceNoteRecorder import kotlinx.coroutines.delay @@ -55,8 +54,7 @@ class ChatMediaLaunchers( @Composable fun rememberChatMediaLaunchers( - inputFieldValue: TextFieldValue, - onInputFieldValueChange: (TextFieldValue) -> Unit, + inputState: TextFieldState, onAddAttachment: (uri: String, name: String, mimeType: String, size: Long) -> Unit, onAddAttachments: (List) -> Unit, onVoiceNoteRecorded: (file: File) -> Unit, @@ -68,8 +66,7 @@ fun rememberChatMediaLaunchers( var isListening by remember { mutableStateOf(false) } var pendingCameraUri by remember { mutableStateOf(null) } - val currentInputFieldValue by rememberUpdatedState(inputFieldValue) - val currentOnInputFieldValueChange by rememberUpdatedState(onInputFieldValueChange) + val currentInputState by rememberUpdatedState(inputState) val currentOnAddAttachment by rememberUpdatedState(onAddAttachment) val currentOnAddAttachments by rememberUpdatedState(onAddAttachments) val currentOnVoiceNoteRecorded by rememberUpdatedState(onVoiceNoteRecorded) @@ -134,14 +131,7 @@ fun rememberChatMediaLaunchers( ?.firstOrNull() .orEmpty() if (spokenText.isNotBlank()) { - val currentText = currentInputFieldValue.text - val merged = - if (currentText.isBlank()) { - spokenText - } else { - "$currentText $spokenText" - } - currentOnInputFieldValueChange(ChatInputPolicy.commandFieldValue(merged)) + currentInputState.appendSpeechComposerDraft(spokenText) } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt index b2e13213..948b891c 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt @@ -58,6 +58,23 @@ private object FullBleedContentType { const val SYSTEM_EVENT: String = "system_event" } +/** Only show the placeholder when there are no messages or renderable live tail items. */ +internal fun shouldShowChatEmptyState( + transcript: TranscriptUiState, + hasReplyError: Boolean, +): Boolean = + transcript.messages.isEmpty() && + transcript.streamingState.streamingMessage == null && + !transcript.isLoading && + !transcript.isAgentTyping && + !hasReplyError && + transcript.clarifyRequest == null && + transcript.vaultUnlockPrompt == null && + transcript.vaultSaveLoginPrompt == null && + transcript.vaultCodePrompt == null && + !transcript.isCompressing && + transcript.compressionStatus == null + /** * The chat message list for FULL-BLEED style (issue #866) — the single chat * surface since the bubble renderer was removed. User messages keep their @@ -105,7 +122,7 @@ fun FullBleedChatList( val isCompressing = transcript.isCompressing val compressionStatus = transcript.compressionStatus val speakingMessageId = transcript.speakingMessageId - if (messages.isEmpty() && !isLoading && !isAgentTyping && replyErrorContent == null) { + if (shouldShowChatEmptyState(transcript, hasReplyError = replyErrorContent != null)) { Box( modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt index d8841774..85b0ceb6 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt @@ -60,6 +60,11 @@ internal fun ChatMessage.isSyntheticSystemRow(): Boolean = displayKind == null && content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER) +private const val HIDDEN_TOOL_NAME = "react_to_message" + +/** Tool rows that never get a transcript bubble. */ +internal fun ChatMessage.isHiddenTool(): Boolean = role == MessageRole.TOOL && toolName == HIDDEN_TOOL_NAME + internal fun ChatMessage.isTimelineMarker(): Boolean = displayKind != null && displayKind !in DisplayKind.nonMarkerKinds /** @@ -108,6 +113,10 @@ fun groupIntoTurns(messages: List): List { agentEntries += AgentEntry.Prose(message) } + message.isHiddenTool() -> { + // Reaction tool calls have no useful bubble; the reaction itself is the feedback. + } + MessageRole.TOOL == message.role -> { agentEntries += AgentEntry.ToolRow(message) } diff --git a/app/src/main/res/values/chat_image_paste.xml b/app/src/main/res/values/chat_image_paste.xml new file mode 100644 index 00000000..63a2530b --- /dev/null +++ b/app/src/main/res/values/chat_image_paste.xml @@ -0,0 +1,8 @@ + + + Adding pasted images… + Could not paste the image. Check access and try again. + Image exceeds the 10 MB attachment limit. + Paste up to 8 images totaling no more than 20 MB. + Wait for the images to finish loading before pasting again. + diff --git a/app/src/test/java/com/m57/hermescontrol/data/model/SessionMessageMultipartTest.kt b/app/src/test/java/com/m57/hermescontrol/data/model/SessionMessageMultipartTest.kt new file mode 100644 index 00000000..e8e6d58a --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/model/SessionMessageMultipartTest.kt @@ -0,0 +1,233 @@ +package com.m57.hermescontrol.data.model + +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonArray +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Regression tests for SessionMessage multipart content and display_content text extraction. + * + * Contract requirements (Issue #1432): + * - [SessionMessage.contentText] and [SessionMessage.displayContentText] extract recognized text parts + * of multipart JsonArray in order. + * - Recognized text parts: {"type": "text", "text": "..."} or {"text": "..."}. + * - Omit "image_url", inline base64 ("data:image/..."), and unknown/malformed parts rather than JSON dumping. + * - Null content -> empty string (""). + * - Absent display_content (null) -> null. + * - Explicit empty string display wins ("" -> ""). + * - Plain string primitives are retained unchanged (including very long plain text). + * - Avoid imposing changes on unknown object tool content (non-array objects dump or retain their object representation). + */ +class SessionMessageMultipartTest { + @Test + fun contentText_whenContentIsNull_returnsEmptyString() { + val message = SessionMessage(content = null) + assertEquals("", message.contentText) + } + + @Test + fun displayContentText_whenDisplayContentIsNull_returnsNull() { + val message = SessionMessage(display_content = null) + assertNull(message.displayContentText) + } + + @Test + fun displayContentText_whenDisplayContentIsExplicitEmptyString_returnsEmptyString() { + val message = SessionMessage(display_content = JsonPrimitive("")) + assertEquals("", message.displayContentText) + } + + @Test + fun contentText_whenContentIsPrimitiveString_retainsUnchanged() { + val text = "Hello world, this is a plain message turn." + val message = SessionMessage(content = JsonPrimitive(text)) + assertEquals(text, message.contentText) + } + + @Test + fun contentText_whenContentIsLongPlainText_retainsEntireStringUnchanged() { + val longText = "A".repeat(100_000) + val message = SessionMessage(content = JsonPrimitive(longText)) + assertEquals(longText, message.contentText) + } + + @Test + fun contentText_whenMultipartWith748kBase64AndImageCaption_extractsCaptionOnly() { + val largeBase64 = "data:image/png;base64," + "iVBORw0KGgoAAAANSUhEUgAA".repeat(31_000) // ~748k chars + val caption = "Check out this screenshot\n@image:/opt/data/images/screen.png" + + val multipart = + buildJsonArray { + add( + buildJsonObject { + put("type", "image_url") + putJsonObject("image_url") { + put("url", largeBase64) + } + }, + ) + add( + buildJsonObject { + put("type", "text") + put("text", caption) + }, + ) + } + + val message = SessionMessage(content = multipart) + assertEquals(caption, message.contentText) + } + + @Test + fun contentText_whenMultipleTextPartsInArray_extractsInOrder() { + val multipart = + buildJsonArray { + add( + buildJsonObject { + put("type", "text") + put("text", "First paragraph.") + }, + ) + add( + buildJsonObject { + put("type", "image_url") + putJsonObject("image_url") { + put("url", "https://example.com/photo.png") + } + }, + ) + add( + buildJsonObject { + put("type", "text") + put("text", "Second paragraph.") + }, + ) + add( + buildJsonObject { + // Shorthand text block without explicit type + put("text", "Third paragraph.") + }, + ) + } + + val message = SessionMessage(content = multipart) + val expected = "First paragraph.\nSecond paragraph.\nThird paragraph." + assertEquals(expected, message.contentText) + } + + @Test + fun contentText_whenMultipartContainsMalformedOrUnknownElements_omitsThemWithoutJsonDumping() { + val multipart = + buildJsonArray { + add(JsonPrimitive(12345)) + add(JsonPrimitive(true)) + add( + buildJsonObject { + put("type", "unknown_blob") + put("raw", "skip_me") + }, + ) + add( + buildJsonObject { + put("type", "text") + put("invalid_key", "no text property here") + }, + ) + add( + buildJsonObject { + put("type", "text") + put("text", "Valid extracted text.") + }, + ) + add( + buildJsonObject { + put("type", "image") + put("source", "base64data") + }, + ) + } + + val message = SessionMessage(content = multipart) + assertEquals("Valid extracted text.", message.contentText) + } + + @Test + fun contentText_whenMultipartHasNoTextParts_returnsEmptyString() { + val multipart = + buildJsonArray { + add( + buildJsonObject { + put("type", "image_url") + putJsonObject("image_url") { + put("url", "https://example.com/icon.png") + } + }, + ) + add( + buildJsonObject { + put("type", "audio") + put("data", "abc123audio") + }, + ) + } + + val message = SessionMessage(content = multipart) + assertEquals("", message.contentText) + } + + @Test + fun displayContentText_whenProjectedDisplayContentIsMultipartArray_extractsTextParts() { + val projectedDisplayArray = + buildJsonArray { + add( + buildJsonObject { + put("type", "text") + put("text", "Summary of conversation") + }, + ) + add( + buildJsonObject { + put("type", "image_url") + putJsonObject("image_url") { + put("url", "data:image/jpeg;base64,12345") + } + }, + ) + add( + buildJsonObject { + put("type", "text") + put("text", "Key takeaways follow.") + }, + ) + } + + val message = + SessionMessage( + content = JsonPrimitive("raw fallback content"), + display_content = projectedDisplayArray, + ) + + assertEquals("Summary of conversation\nKey takeaways follow.", message.displayContentText) + } + + @Test + fun contentText_whenContentIsJsonObjectToolCallOrResult_doesNotImposeMultipartChanges() { + val toolObject = + buildJsonObject { + put("status", "success") + put("output", "tool execution output") + } + + val message = SessionMessage(content = toolObject) + // Ensure non-array JsonObject continues existing behavior (e.g. toString/not mangled as multipart array) + assertEquals(toolObject.toString(), message.contentText) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/GatewayContractFramesTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/GatewayContractFramesTest.kt index b92d4208..3794234d 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/GatewayContractFramesTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/GatewayContractFramesTest.kt @@ -71,14 +71,20 @@ class GatewayContractFramesTest { @After fun tearDown() { - HermesWsClient.releaseExternalActivityConnectionLease() - HermesWsClient.releaseBackgroundConnectionLease() - HermesWsClient.disconnect(clearPendingMessages = true) - runBlocking { - withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } } + try { + HermesWsClient.releaseExternalActivityConnectionLease() + HermesWsClient.releaseBackgroundConnectionLease() + HermesWsClient.disconnect(clearPendingMessages = true) + runBlocking { + withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } } + } + } finally { + try { + mockWebServer.shutdown() + } finally { + unmockkAll() + } } - runCatching { mockWebServer.shutdown() } - unmockkAll() } /** Connect, run [trigger], and return the first frame the server sees for [method]. */ @@ -91,7 +97,7 @@ class GatewayContractFramesTest { val captured = CountDownLatch(1) val frame = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/GatewayContractTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/GatewayContractTest.kt index be06f9c6..6a66b8bb 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/GatewayContractTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/GatewayContractTest.kt @@ -105,7 +105,7 @@ class GatewayContractTest { // ChatViewModel, GroupChatViewModel "session.create" to setOf("profile", "title", "source", "hidden"), // ChatViewModel.resumeSession, NotificationReplyReceiver - "session.resume" to setOf("session_id", "omit_messages"), + "session.resume" to setOf("session_id", "source", "omit_messages"), // ChatViewModel, GroupChatViewModel "session.interrupt" to setOf("session_id"), ) diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientSessionControlTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientSessionControlTest.kt index 4d8aeec8..7582e178 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientSessionControlTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientSessionControlTest.kt @@ -76,15 +76,21 @@ class HermesWsClientSessionControlTest { @After fun tearDown() { - AuthManager.resetAuthStateForTest() - HermesWsClient.releaseExternalActivityConnectionLease() - HermesWsClient.releaseBackgroundConnectionLease() - HermesWsClient.disconnect(clearPendingMessages = true) - runBlocking { - withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } } + try { + AuthManager.resetAuthStateForTest() + HermesWsClient.releaseExternalActivityConnectionLease() + HermesWsClient.releaseBackgroundConnectionLease() + HermesWsClient.disconnect(clearPendingMessages = true) + runBlocking { + withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } } + } + } finally { + try { + mockWebServer.shutdown() + } finally { + unmockkAll() + } } - runCatching { mockWebServer.shutdown() } - unmockkAll() } private fun connectClient() { @@ -101,7 +107,7 @@ class HermesWsClientSessionControlTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -163,7 +169,7 @@ class HermesWsClientSessionControlTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -224,7 +230,7 @@ class HermesWsClientSessionControlTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt index 7b5c1331..e6e1856f 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt @@ -104,18 +104,17 @@ class HermesWsClientTest { @After fun tearDown() { - HermesWsClient.releaseExternalActivityConnectionLease() - HermesWsClient.releaseBackgroundConnectionLease() - HermesWsClient.disconnect(clearPendingMessages = true) - // Wait a bit to allow internal OkHttp coroutines to clean up before shutting down MockWebServer - // Increased from 100ms for OkHttp 5.x — needs more time for the WS close handshake - Thread.sleep(500) try { - mockWebServer.shutdown() - } catch (e: Throwable) { - e.printStackTrace() + HermesWsClient.releaseExternalActivityConnectionLease() + HermesWsClient.releaseBackgroundConnectionLease() + HermesWsClient.disconnect(clearPendingMessages = true) + } finally { + try { + mockWebServer.shutdown() + } finally { + unmockkAll() + } } - unmockkAll() } @Test @@ -126,7 +125,7 @@ class HermesWsClientTest { var receivedMessage: String? = null mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -171,7 +170,7 @@ class HermesWsClientTest { var capabilityFrame: String? = null mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -221,7 +220,7 @@ class HermesWsClientTest { val capabilityResponseCount = AtomicInteger(0) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -301,7 +300,7 @@ class HermesWsClientTest { fun testOpenRequestsReplayUsesTheLiveServerRequestDispatcher() = runBlocking { mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -759,7 +758,7 @@ class HermesWsClientTest { HermesWsClient.connectedForTest.set(true) val oldSocket = mockk(relaxed = true) HermesWsClient.webSocketForTest = oldSocket - mockWebServer.enqueue(MockResponse().withWebSocketUpgrade(object : WebSocketListener() {})) + mockWebServer.enqueue(MockResponse().withClosingWebSocketUpgrade(object : WebSocketListener() {})) HermesWsClient.connect() @@ -789,7 +788,7 @@ class HermesWsClientTest { assertTrue(refreshStarted.await(5, TimeUnit.SECONDS)) HermesWsClient.disconnect(clearPendingMessages = true) - mockWebServer.enqueue(MockResponse().withWebSocketUpgrade(object : WebSocketListener() {})) + mockWebServer.enqueue(MockResponse().withClosingWebSocketUpgrade(object : WebSocketListener() {})) val disconnectedGeneration = HermesWsClient.connectionGenerationForTest.get() val replacementConnect = thread { HermesWsClient.connect() } val generationDeadline = System.currentTimeMillis() + 5_000 @@ -821,7 +820,7 @@ class HermesWsClientTest { val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -862,7 +861,7 @@ class HermesWsClientTest { @Test fun testBackgroundWithoutPendingWorkDisconnects() { - mockWebServer.enqueue(MockResponse().withWebSocketUpgrade(object : WebSocketListener() {})) + mockWebServer.enqueue(MockResponse().withClosingWebSocketUpgrade(object : WebSocketListener() {})) HermesWsClient.connect() runBlocking { withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.CONNECTED } } } @@ -874,7 +873,7 @@ class HermesWsClientTest { @Test fun testExternalActivityLeaseKeepsIdleBackgroundSocketUntilReleased() { - mockWebServer.enqueue(MockResponse().withWebSocketUpgrade(object : WebSocketListener() {})) + mockWebServer.enqueue(MockResponse().withClosingWebSocketUpgrade(object : WebSocketListener() {})) HermesWsClient.connect() runBlocking { withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.CONNECTED } } } @@ -891,7 +890,7 @@ class HermesWsClientTest { @Test fun testBackgroundConnectionLeaseKeepsIdleBackgroundSocketUntilReleased() { - mockWebServer.enqueue(MockResponse().withWebSocketUpgrade(object : WebSocketListener() {})) + mockWebServer.enqueue(MockResponse().withClosingWebSocketUpgrade(object : WebSocketListener() {})) HermesWsClient.connect() runBlocking { withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.CONNECTED } } } @@ -908,7 +907,7 @@ class HermesWsClientTest { @Test fun testBackgroundWithPendingReplyStaysConnected() { - mockWebServer.enqueue(MockResponse().withWebSocketUpgrade(object : WebSocketListener() {})) + mockWebServer.enqueue(MockResponse().withClosingWebSocketUpgrade(object : WebSocketListener() {})) HermesWsClient.connect() runBlocking { withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.CONNECTED } } } HermesWsClient.sendMessage("session-1", "hello") @@ -923,7 +922,7 @@ class HermesWsClientTest { lateinit var serverSocket: WebSocket val connectedLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -958,7 +957,7 @@ class HermesWsClientTest { fun testBackgroundQueuedSendDisconnectsAfterFlush() { val messageLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onMessage( webSocket: WebSocket, @@ -987,7 +986,7 @@ class HermesWsClientTest { lateinit var serverSocket: WebSocket val connectedLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1033,7 +1032,7 @@ class HermesWsClientTest { val closedLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1073,7 +1072,7 @@ class HermesWsClientTest { var receivedMessage: String? = null mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1128,7 +1127,7 @@ class HermesWsClientTest { val connect2Latch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1142,7 +1141,7 @@ class HermesWsClientTest { ) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1206,7 +1205,7 @@ class HermesWsClientTest { val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( ws: WebSocket, @@ -1235,7 +1234,7 @@ class HermesWsClientTest { val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( ws: WebSocket, @@ -1265,7 +1264,7 @@ class HermesWsClientTest { fun testDoubleConnect_ignoresSecondCallWhenConnected() { val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( ws: WebSocket, @@ -1291,7 +1290,7 @@ class HermesWsClientTest { fun testStatusTransitionOnConnect() { val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( ws: WebSocket, @@ -1328,7 +1327,7 @@ class HermesWsClientTest { val connectLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( ws: WebSocket, @@ -1342,7 +1341,7 @@ class HermesWsClientTest { // Enqueue a second response for reconnect attempt mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( ws: WebSocket, @@ -1400,7 +1399,7 @@ class HermesWsClientTest { val connectLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( ws: WebSocket, @@ -1537,7 +1536,7 @@ class HermesWsClientTest { var serverWebSocket: WebSocket? = null val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1589,7 +1588,7 @@ class HermesWsClientTest { var serverWebSocket: WebSocket? = null val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1664,7 +1663,7 @@ class HermesWsClientTest { var serverWebSocket: WebSocket? = null val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1719,7 +1718,7 @@ class HermesWsClientTest { } mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1780,7 +1779,7 @@ class HermesWsClientTest { val socket = mockk(relaxed = true) HermesWsClient.intentionalCloseForTest.set(false) val listener = HermesWsClient.createListenerForTest(HermesWsClient.connectionGenerationForTest.get()) - // Directly install the reflection-built listener so onMessage runs even + // Directly install the transport listener so onMessage runs even // though this test never performed a real OkHttp connect. HermesWsClient.webSocketForTest = socket mockkObject(EventParser) @@ -1826,7 +1825,7 @@ class HermesWsClientTest { } mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1888,7 +1887,7 @@ class HermesWsClientTest { var pingReceived = false mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -1971,7 +1970,7 @@ class HermesWsClientTest { val serverLatch = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTypedCallTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTypedCallTest.kt index b8c7d01b..6bc669eb 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTypedCallTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTypedCallTest.kt @@ -103,15 +103,21 @@ class HermesWsClientTypedCallTest { @After fun tearDown() { - AuthManager.resetAuthStateForTest() - HermesWsClient.releaseExternalActivityConnectionLease() - HermesWsClient.releaseBackgroundConnectionLease() - HermesWsClient.disconnect(clearPendingMessages = true) - runBlocking { - withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } } + try { + AuthManager.resetAuthStateForTest() + HermesWsClient.releaseExternalActivityConnectionLease() + HermesWsClient.releaseBackgroundConnectionLease() + HermesWsClient.disconnect(clearPendingMessages = true) + runBlocking { + withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } } + } + } finally { + try { + mockWebServer.shutdown() + } finally { + unmockkAll() + } } - runCatching { mockWebServer.shutdown() } - unmockkAll() } private fun connectClient() { @@ -128,7 +134,7 @@ class HermesWsClientTypedCallTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -190,7 +196,7 @@ class HermesWsClientTypedCallTest { val opened = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -233,7 +239,7 @@ class HermesWsClientTypedCallTest { val opened = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -281,7 +287,7 @@ class HermesWsClientTypedCallTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -332,7 +338,7 @@ class HermesWsClientTypedCallTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -382,7 +388,7 @@ class HermesWsClientTypedCallTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -429,7 +435,7 @@ class HermesWsClientTypedCallTest { val opened = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -475,7 +481,7 @@ class HermesWsClientTypedCallTest { val requestReceived = CountDownLatch(1) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTypedSendTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTypedSendTest.kt index 655e174d..5bd4faba 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTypedSendTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTypedSendTest.kt @@ -77,15 +77,21 @@ class HermesWsClientTypedSendTest { @After fun tearDown() { - AuthManager.resetAuthStateForTest() - HermesWsClient.releaseExternalActivityConnectionLease() - HermesWsClient.releaseBackgroundConnectionLease() - HermesWsClient.disconnect(clearPendingMessages = true) - runBlocking { - withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } } + try { + AuthManager.resetAuthStateForTest() + HermesWsClient.releaseExternalActivityConnectionLease() + HermesWsClient.releaseBackgroundConnectionLease() + HermesWsClient.disconnect(clearPendingMessages = true) + runBlocking { + withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } } + } + } finally { + try { + mockWebServer.shutdown() + } finally { + unmockkAll() + } } - runCatching { mockWebServer.shutdown() } - unmockkAll() } private fun connectClient() { @@ -102,7 +108,7 @@ class HermesWsClientTypedSendTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -163,7 +169,7 @@ class HermesWsClientTypedSendTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -216,7 +222,7 @@ class HermesWsClientTypedSendTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -274,7 +280,7 @@ class HermesWsClientTypedSendTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -325,7 +331,7 @@ class HermesWsClientTypedSendTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -382,7 +388,7 @@ class HermesWsClientTypedSendTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, @@ -437,7 +443,7 @@ class HermesWsClientTypedSendTest { val frameRef = AtomicReference(null) mockWebServer.enqueue( - MockResponse().withWebSocketUpgrade( + MockResponse().withClosingWebSocketUpgrade( object : WebSocketListener() { override fun onOpen( webSocket: WebSocket, diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanup.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanup.kt new file mode 100644 index 00000000..e869a090 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanup.kt @@ -0,0 +1,56 @@ +package com.m57.hermescontrol.data.ws + +import okhttp3.Response +import okhttp3.WebSocket +import okhttp3.WebSocketListener +import okhttp3.mockwebserver.MockResponse +import okio.ByteString + +/** + * Keep the test listener's behavior, but acknowledge peer close frames so MockWebServer's + * WebSocket task can finish. WebSocketListener's default onClosing is a no-op. + */ +internal fun MockResponse.withClosingWebSocketUpgrade(delegate: WebSocketListener): MockResponse = + withWebSocketUpgrade( + object : WebSocketListener() { + override fun onOpen( + webSocket: WebSocket, + response: Response, + ) = delegate.onOpen(webSocket, response) + + override fun onMessage( + webSocket: WebSocket, + text: String, + ) = delegate.onMessage(webSocket, text) + + override fun onMessage( + webSocket: WebSocket, + bytes: ByteString, + ) = delegate.onMessage(webSocket, bytes) + + override fun onClosing( + webSocket: WebSocket, + code: Int, + reason: String, + ) { + try { + delegate.onClosing(webSocket, code, reason) + } finally { + // The delegate may already have initiated a close; a false return is valid. + webSocket.close(code, reason) + } + } + + override fun onClosed( + webSocket: WebSocket, + code: Int, + reason: String, + ) = delegate.onClosed(webSocket, code, reason) + + override fun onFailure( + webSocket: WebSocket, + t: Throwable, + response: Response?, + ) = delegate.onFailure(webSocket, t, response) + }, + ) diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanupListenerTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanupListenerTest.kt new file mode 100644 index 00000000..383b102e --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanupListenerTest.kt @@ -0,0 +1,113 @@ +package com.m57.hermescontrol.data.ws + +import io.mockk.every +import io.mockk.mockk +import io.mockk.verifySequence +import okhttp3.Response +import okhttp3.WebSocket +import okhttp3.WebSocketListener +import okhttp3.mockwebserver.MockResponse +import okio.ByteString.Companion.encodeUtf8 +import org.junit.Assert.assertSame +import org.junit.Assert.assertThrows +import org.junit.Test + +class MockWebSocketCleanupListenerTest { + private val socket = mockk() + private val delegate = mockk(relaxed = true) + private val listener: WebSocketListener + get() = requireNotNull(MockResponse().withClosingWebSocketUpgrade(delegate).webSocketListener) + + @Test + fun acknowledgesCloseAfterDelegatedCallback() { + every { socket.close(1000, "complete") } returns true + + listener.onClosing(socket, 1000, "complete") + + verifySequence { + delegate.onClosing(socket, 1000, "complete") + socket.close(1000, "complete") + } + } + + @Test + fun acknowledgesCloseEvenWhenDelegatedCallbackThrows() { + val failure = IllegalStateException("callback failure") + every { delegate.onClosing(socket, 1000, "complete") } throws failure + every { socket.close(1000, "complete") } returns true + + val thrown = + assertThrows(IllegalStateException::class.java) { + listener.onClosing(socket, 1000, "complete") + } + + assertSame(failure, thrown) + verifySequence { + delegate.onClosing(socket, 1000, "complete") + socket.close(1000, "complete") + } + } + + @Test + fun acceptsAlreadyInitiatedCustomCloseWithoutOverridingIt() { + every { socket.close(1001, "custom close") } returns true + every { socket.close(1000, "complete") } returns false + every { delegate.onClosing(socket, 1000, "complete") } answers { + socket.close(1001, "custom close") + Unit + } + + listener.onClosing(socket, 1000, "complete") + + verifySequence { + delegate.onClosing(socket, 1000, "complete") + socket.close(1001, "custom close") + socket.close(1000, "complete") + } + } + + @Test + fun forwardsOpenWithOriginalSocketAndResponse() { + val response = mockk() + + listener.onOpen(socket, response) + + verifySequence { delegate.onOpen(socket, response) } + } + + @Test + fun forwardsBothTextAndBinaryMessages() { + val bytes = "binary message".encodeUtf8() + val wrapped = listener + + wrapped.onMessage(socket, "text message") + wrapped.onMessage(socket, bytes) + + verifySequence { + delegate.onMessage(socket, "text message") + delegate.onMessage(socket, bytes) + } + } + + @Test + fun forwardsClosedWithOriginalCodeAndReason() { + listener.onClosed(socket, 1001, "server close") + + verifySequence { delegate.onClosed(socket, 1001, "server close") } + } + + @Test + fun forwardsFailureWithOriginalThrowableAndNullableResponse() { + val failure = IllegalStateException("transport failure") + val response = mockk() + val wrapped = listener + + wrapped.onFailure(socket, failure, response) + wrapped.onFailure(socket, failure, null) + + verifySequence { + delegate.onFailure(socket, failure, response) + delegate.onFailure(socket, failure, null) + } + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanupTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanupTest.kt new file mode 100644 index 00000000..b7499557 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/MockWebSocketCleanupTest.kt @@ -0,0 +1,104 @@ +package com.m57.hermescontrol.data.ws + +import android.util.Log +import io.mockk.every +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.Response +import okhttp3.WebSocket +import okhttp3.WebSocketListener +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference + +class MockWebSocketCleanupTest { + @Before + fun setUp() { + mockkStatic(Log::class) + every { Log.isLoggable(any(), any()) } returns false + every { Log.println(any(), any(), any()) } returns 0 + } + + @After + fun tearDown() { + unmockkAll() + } + + @Test + fun peerCloseAllowsMockServerShutdownWithoutLeavingRunningTasks() { + val server = MockWebServer() + val client = OkHttpClient() + val serverSocket = AtomicReference() + val opened = CountDownLatch(1) + val closing = CountDownLatch(1) + val closed = CountDownLatch(1) + server.enqueue( + MockResponse().withClosingWebSocketUpgrade( + object : WebSocketListener() { + override fun onOpen( + webSocket: WebSocket, + response: Response, + ) { + serverSocket.set(webSocket) + } + + override fun onClosing( + webSocket: WebSocket, + code: Int, + reason: String, + ) { + closing.countDown() + } + }, + ), + ) + server.start() + val socket = + client.newWebSocket( + Request.Builder().url(server.url("/")).build(), + object : WebSocketListener() { + override fun onOpen( + webSocket: WebSocket, + response: Response, + ) { + opened.countDown() + } + + override fun onClosed( + webSocket: WebSocket, + code: Int, + reason: String, + ) { + closed.countDown() + } + }, + ) + try { + assertTrue("The WebSocket must open", opened.await(5, TimeUnit.SECONDS)) + assertTrue(socket.close(1000, "test complete")) + assertTrue("The mock peer must observe the close", closing.await(5, TimeUnit.SECONDS)) + // Complete the handshake before shutdown stops the server's task runner. + assertTrue("The close handshake must complete", closed.await(5, TimeUnit.SECONDS)) + // No fixed sleep or swallowed shutdown exception: an active server queue fails this regression. + server.shutdown() + } finally { + socket.cancel() + // MockWebServer's accepted socket has no outbound Call, so cancel() is not supported. + serverSocket.get()?.close(1000, "cleanup") + try { + server.shutdown() + } finally { + client.dispatcher.executorService.shutdown() + client.connectionPool.evictAll() + } + } + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/contract/SessionPromptTypesTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/contract/SessionPromptTypesTest.kt index bf5c9c44..60f700cd 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/contract/SessionPromptTypesTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/contract/SessionPromptTypesTest.kt @@ -118,6 +118,48 @@ class SessionPromptTypesTest { assertEquals(setOf("session_id", "omit_messages", "profile"), (encoded as JsonObject).keys) } + @Test + fun resumeWithDesktopSourceEncodesSourceKey() { + val params = + SessionResumeParams( + sessionId = "s-457", + source = DESKTOP_SESSION_SOURCE, + omitMessages = true, + ) + val encoded = OkHttpProvider.json.encodeToJsonElement(SessionResumeParams.serializer(), params) + + val expected = + buildJsonObject { + put("session_id", "s-457") + put("source", "desktop") + put("omit_messages", true) + } + + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertEquals(setOf("session_id", "source", "omit_messages"), (encoded as JsonObject).keys) + } + + @Test + fun resumeOmitsSourceWhenUnset() { + val encoded = + OkHttpProvider.json.encodeToJsonElement( + SessionResumeParams.serializer(), + SessionResumeParams(sessionId = "s-458"), + ) + + assertTrue(encoded is JsonObject) + assertNull((encoded as JsonObject)["source"]) + } + + @Test + fun createAndResumeDeclareTheSameDesktopSource() { + // #1450: create passed source and resume did not, so the gateway resolved the resumed + // runtime from its host env ("tui") and staged a bogus surface switch. Same constant, + // so the two call sites cannot drift again. + assertEquals("desktop", DESKTOP_SESSION_SOURCE) + } + @Test fun resumeWithExplicitFalseOmitMessagesIsEncoded() { val params = SessionResumeParams(sessionId = "s-789", omitMessages = false) diff --git a/app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt b/app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt index e9b0aaa3..4e39516b 100644 --- a/app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt @@ -330,7 +330,13 @@ class NotificationReplyReceiverTest { verify { HermesWsClient.request( WsMethods.SESSION_RESUME, - match { it.str("session_id") == "session-abc" && it["omit_messages"] == JsonPrimitive(true) }, + match { + it.str("session_id") == "session-abc" && + it["omit_messages"] == JsonPrimitive(true) && + // #1450: source must match session.create, or the gateway resolves the + // resumed runtime from its host env and stages a bogus surface switch. + it["source"] == JsonPrimitive("desktop") + }, any(), ) HermesWsClient.request( diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatLiveOrderingRegressionTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatLiveOrderingRegressionTest.kt new file mode 100644 index 00000000..50041d78 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatLiveOrderingRegressionTest.kt @@ -0,0 +1,464 @@ +package com.m57.hermescontrol.ui.chat + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Test + +/** + * Regression coverage for issue #1451: + * Transcript ordering when REST confirms a newer prompt while older live rows + * (observed live user/tool + review notices) remain unmatched/unconfirmed. + */ +class ChatLiveOrderingRegressionTest { + @Test + fun unconfirmedLiveUserAndToolWithReviewNoticesStayBeforeConfirmedNewerPrompt() { + // Given an observed live history: + // 1. Live user prompt (unconfirmed by REST) + // 2. Live tool call / result (unconfirmed by REST) + // 3. First system review notice + // 4. Second system review notice + // 5. Later live user prompt + val liveUserOld = + ChatMessage( + id = "live-user-old", + role = MessageRole.USER, + content = "first task", + timestamp = 1_000L, + ) + val liveTool = + ChatMessage( + id = "live-tool-1", + role = MessageRole.TOOL, + content = """{"result":{"output":"task done","exit_code":0}}""", + toolName = "terminal", + toolCallId = "call_task_1", + timestamp = 1_100L, + ) + val systemReview1 = + ChatMessage( + id = "sys-review-1", + role = MessageRole.SYSTEM, + content = "💾 Self-improvement review: skill updated", + timestamp = 1_200L, + ) + val systemReview2 = + ChatMessage( + id = "sys-review-2", + role = MessageRole.SYSTEM, + content = "💾 Self-improvement review: playbook refreshed", + timestamp = 1_300L, + ) + val liveUserNew = + ChatMessage( + id = "live-user-new", + role = MessageRole.USER, + content = "second prompt", + timestamp = 1_400L, + ) + + val currentMessages = + listOf( + liveUserOld, + liveTool, + systemReview1, + systemReview2, + liveUserNew, + ) + + // REST sync only returns/confirms the second prompt (e.g. rest-s-5), + // while the older live user/tool and review notices remain unmatched in REST. + val restConfirmedNew = + ChatMessage( + id = "rest-s-5", + role = MessageRole.USER, + content = "second prompt", + timestamp = 1_450L, + ) + val restMessages = listOf(restConfirmedNew) + + // When merged with chronological = true, preserveLiveIds = false + val merged = + mergeTranscriptWithLive( + restMessages = restMessages, + currentMessages = currentMessages, + chronological = true, + preserveLiveIds = false, + ) + + // Then: + // 1. All rows must be preserved. + assertEquals(5, merged.size) + + // 2. Ordering contract: the older live user, tool, and two system review notices + // MUST remain before the newer confirmed prompt. + val expectedIds = + listOf( + "live-user-old", + "live-tool-1", + "sys-review-1", + "sys-review-2", + "live-user-new", // Merged with rest-s-5; when role=USER, match's ID is preserved, restId recorded + ) + assertEquals(expectedIds, merged.map { it.id }) + + // 3. The confirmed prompt acquires its canonical REST identity + val confirmedRow = merged.last() + assertEquals("rest-s-5", confirmedRow.canonicalRestId) + assertEquals(MessageRole.USER, confirmedRow.role) + assertEquals("second prompt", confirmedRow.content) + + // 4. Older unconfirmed rows must NOT invent or acquire rest identities + val unconfirmedRows = merged.take(4) + unconfirmedRows.forEach { row -> + assertNotEquals("rest-s-5", row.restId) + assertEquals(null, row.canonicalRestId) + } + + // 5. Repeated merge must be idempotent + val repeatedMerge = + mergeTranscriptWithLive( + restMessages = restMessages, + currentMessages = merged, + chronological = true, + preserveLiveIds = false, + ) + assertEquals(merged.map { it.id }, repeatedMerge.map { it.id }) + assertEquals(merged, repeatedMerge) + } + + @Test + fun controlPlainAppendAndNonConfirmationMaintainsObservedLiveOrder() { + // Control scenario: + // Current live transcript contains an old user prompt, tool call, two review notices, + // and a later user prompt. + // REST returns no confirmations (emptyList) or an unmatching server message. + val liveUserOld = + ChatMessage( + id = "live-user-old", + role = MessageRole.USER, + content = "first task", + timestamp = 1_000L, + ) + val liveTool = + ChatMessage( + id = "live-tool-1", + role = MessageRole.TOOL, + content = """{"result":{"output":"task done","exit_code":0}}""", + toolName = "terminal", + toolCallId = "call_task_1", + timestamp = 1_100L, + ) + val systemReview1 = + ChatMessage( + id = "sys-review-1", + role = MessageRole.SYSTEM, + content = "💾 Self-improvement review: note 1", + timestamp = 1_200L, + ) + val systemReview2 = + ChatMessage( + id = "sys-review-2", + role = MessageRole.SYSTEM, + content = "💾 Self-improvement review: note 2", + timestamp = 1_300L, + ) + val liveUserNew = + ChatMessage( + id = "live-user-new", + role = MessageRole.USER, + content = "second prompt", + timestamp = 1_400L, + ) + + val currentMessages = + listOf( + liveUserOld, + liveTool, + systemReview1, + systemReview2, + liveUserNew, + ) + + // Case A: REST sync is empty + val mergedEmpty = + mergeTranscriptWithLive( + restMessages = emptyList(), + currentMessages = currentMessages, + chronological = true, + preserveLiveIds = false, + ) + assertEquals( + listOf( + "live-user-old", + "live-tool-1", + "sys-review-1", + "sys-review-2", + "live-user-new", + ), + mergedEmpty.map { + it.id + }, + ) + + // Case B: REST returns an unrelated / earlier server message (e.g. session start rest-s-0) + val canonicalPrior = + ChatMessage( + id = "rest-s-0", + role = MessageRole.USER, + content = "very first conversation starter", + timestamp = 500L, + ) + val mergedWithPrior = + mergeTranscriptWithLive( + restMessages = listOf(canonicalPrior), + currentMessages = currentMessages, + chronological = true, + preserveLiveIds = false, + ) + + // Canonical server rows maintain server order, while observed live unconfirmed block stays intact + assertEquals( + listOf("rest-s-0", "live-user-old", "live-tool-1", "sys-review-1", "sys-review-2", "live-user-new"), + mergedWithPrior.map { it.id }, + ) + + // Idempotence check on control + val idempotentMergedWithPrior = + mergeTranscriptWithLive( + restMessages = listOf(canonicalPrior), + currentMessages = mergedWithPrior, + chronological = true, + preserveLiveIds = false, + ) + assertEquals(mergedWithPrior.map { it.id }, idempotentMergedWithPrior.map { it.id }) + } + + @Test + fun sessionStartMarkerRemainsBeforeOldUnresolvedUserToolReviewBlockWhenNewerUserConfirmsAsRestS0() { + val sessionStart = + ChatMessage( + id = "system-session-start", + role = MessageRole.SYSTEM, + content = "Session created", + timestamp = 100L, + ) + val liveUserOld = + ChatMessage( + id = "live-user-old", + role = MessageRole.USER, + content = "unconfirmed first prompt", + timestamp = 200L, + ) + val liveTool = + ChatMessage( + id = "live-tool-1", + role = MessageRole.TOOL, + content = """{"result":"done"}""", + toolName = "terminal", + toolCallId = "call_1", + timestamp = 300L, + ) + val sysReview = + ChatMessage( + id = "sys-review-1", + role = MessageRole.SYSTEM, + content = "💾 Self-improvement review: note", + timestamp = 400L, + ) + val liveUserNew = + ChatMessage( + id = "live-user-new", + role = MessageRole.USER, + content = "confirmed second prompt", + timestamp = 500L, + ) + + val currentMessages = + listOf( + sessionStart, + liveUserOld, + liveTool, + sysReview, + liveUserNew, + ) + + // Newer observed USER confirms as canonical index 0 (rest-s-0) + val restConfirmedNew = + ChatMessage( + id = "rest-s-0", + role = MessageRole.USER, + content = "confirmed second prompt", + timestamp = 550L, + ) + + val merged = + mergeTranscriptWithLive( + restMessages = listOf(restConfirmedNew), + currentMessages = currentMessages, + chronological = true, + preserveLiveIds = false, + ) + + val expectedIds = + listOf( + "system-session-start", + "live-user-old", + "live-tool-1", + "sys-review-1", + "live-user-new", + ) + assertEquals(expectedIds, merged.map { it.id }) + assertEquals("rest-s-0", merged.last().canonicalRestId) + } + + @Test + fun canonicalRowsRemainServerOrderedAcrossGapsWithOldLiveBlockBeforeNextConfirmedUserAcrossSkewedTimestamps() { + // Skewed/inverted timestamps across rows + val canonicalPrior = + ChatMessage( + id = "rest-s-2", + role = MessageRole.USER, + content = "prior prompt", + timestamp = 8_000L, // skewed higher timestamp than later turns + ) + val liveUserOld = + ChatMessage( + id = "live-user-old", + role = MessageRole.USER, + content = "unresolved old prompt", + timestamp = 2_000L, + ) + val liveTool = + ChatMessage( + id = "live-tool-1", + role = MessageRole.TOOL, + content = """{"result":"ok"}""", + toolName = "terminal", + toolCallId = "call_skew", + timestamp = 2_000L, // equal timestamp + ) + val sysReview = + ChatMessage( + id = "sys-review-1", + role = MessageRole.SYSTEM, + content = "💾 Self-improvement review: updated", + timestamp = 1_000L, // earlier timestamp + ) + val liveUserNew = + ChatMessage( + id = "live-user-new", + role = MessageRole.USER, + content = "next user prompt", + timestamp = 3_000L, + ) + + val currentMessages = + listOf( + canonicalPrior, + liveUserOld, + liveTool, + sysReview, + liveUserNew, + ) + + // Gap in canonical sequence: rest-s-2 then rest-s-9 (gap 3..8 missing/unloaded) + val canonicalFollow = + ChatMessage( + id = "rest-s-9", + role = MessageRole.USER, + content = "next user prompt", + timestamp = 2_500L, // skewed timestamp compared to rest-s-2 + ) + val restMessages = listOf(canonicalPrior, canonicalFollow) + + val merged = + mergeTranscriptWithLive( + restMessages = restMessages, + currentMessages = currentMessages, + chronological = true, + preserveLiveIds = false, + ) + + val expectedIds = + listOf( + "rest-s-2", + "live-user-old", + "live-tool-1", + "sys-review-1", + "live-user-new", + ) + assertEquals(expectedIds, merged.map { it.id }) + + // Repeated merge must remain identical + val repeatedMerge = + mergeTranscriptWithLive( + restMessages = restMessages, + currentMessages = merged, + chronological = true, + preserveLiveIds = false, + ) + assertEquals(merged.map { it.id }, repeatedMerge.map { it.id }) + assertEquals(merged, repeatedMerge) + } + + @Test + fun assistantConfirmationDoesNotPullUnresolvedUserOutOfPendingTail() { + val unresolvedUser = + ChatMessage( + id = "live-user-unresolved", + role = MessageRole.USER, + content = "prompt not matching rest", + timestamp = 1_000L, + ) + val liveTool = + ChatMessage( + id = "live-tool-1", + role = MessageRole.TOOL, + content = """{"result":"in progress"}""", + toolName = "terminal", + toolCallId = "call_assistant_test", + timestamp = 1_100L, + ) + val liveAssistant = + ChatMessage( + id = "live-assistant-1", + role = MessageRole.ASSISTANT, + content = "Here is the response", + timestamp = 1_200L, + ) + + val currentMessages = + listOf( + unresolvedUser, + liveTool, + liveAssistant, + ) + + // REST confirms only the ASSISTANT message as rest-s-4 (e.g. user was dropped or mismatched) + val restConfirmedAssistant = + ChatMessage( + id = "rest-s-4", + role = MessageRole.ASSISTANT, + content = "Here is the response", + timestamp = 1_200L, + ) + + val merged = + mergeTranscriptWithLive( + restMessages = listOf(restConfirmedAssistant), + currentMessages = currentMessages, + chronological = true, + preserveLiveIds = false, + ) + + // Assistant confirmation alone must not anchor/pull the unresolved user or tool + // before rest-s-4. The confirmed assistant appears first, unresolved user/tool remain in pending tail. + val expectedIds = + listOf( + "rest-s-4", + "live-user-unresolved", + "live-tool-1", + ) + assertEquals(expectedIds, merged.map { it.id }) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPastedImageStoreTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPastedImageStoreTest.kt new file mode 100644 index 00000000..c23e5517 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPastedImageStoreTest.kt @@ -0,0 +1,253 @@ +package com.m57.hermescontrol.ui.chat + +import kotlinx.coroutines.CancellationException +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.ByteArrayInputStream +import java.io.IOException +import java.io.InputStream +import java.util.UUID +import java.util.concurrent.Callable +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit + +class ChatPastedImageStoreTest { + @get:Rule val temp = TemporaryFolder() + + @Test + fun startupSweepDeletesOnlyPastedImagesOlderThanSevenDays() { + val cacheDir = temp.newFolder("cache") + val directory = cacheDir.resolve("pasted_images").apply { mkdir() } + val now = 2_000_000_000_000L + val cutoff = now - TimeUnit.DAYS.toMillis(7) + val stale = stagePastedImage(ByteArrayInputStream(byteArrayOf(1)), directory, "png")!! + assertTrue(stale.setLastModified(cutoff - 1000)) + val boundary = directory.resolve("boundary.png").apply { writeText("keep") } + assertTrue(boundary.setLastModified(cutoff)) + val recent = directory.resolve("recent.png").apply { writeText("keep") } + assertTrue(recent.setLastModified(now)) + val future = directory.resolve("future.png").apply { writeText("keep") } + assertTrue(future.setLastModified(now + 1000)) + val unrelated = cacheDir.resolve("unrelated.png").apply { writeText("keep") } + assertTrue(unrelated.setLastModified(cutoff - 1000)) + val nested = + directory + .resolve("nested") + .apply { mkdir() } + .resolve("old.png") + .apply { writeText("keep") } + assertTrue(nested.setLastModified(cutoff - 1000)) + + cleanStalePastedImages(cacheDir, now) + cleanStalePastedImages(cacheDir, now) + + assertFalse(stale.exists()) + listOf(boundary, recent, future, unrelated, nested).forEach { assertEquals("keep", it.readText()) } + } + + @Test + fun startupSweepDoesNotCreateAMissingCacheDirectory() { + val cacheDir = temp.newFolder("empty-cache") + + cleanStalePastedImages(cacheDir) + + assertTrue(cacheDir.listFiles()!!.isEmpty()) + } + + @Test + fun startupSweepPreservesAnImportCreatedAfterTheStartupSnapshot() { + val cacheDir = temp.newFolder("live-cache") + val directory = cacheDir.resolve("pasted_images") + val startupTime = System.currentTimeMillis() + val imported = stagePastedImage(ByteArrayInputStream(byteArrayOf(1, 2)), directory, "png")!! + + cleanStalePastedImages(cacheDir, startupTime) + + assertArrayEquals(byteArrayOf(1, 2), imported.readBytes()) + } + + @Test + fun exactLimitIsAcceptedWithItsActualByteCount() { + val dir = temp.newFolder("exact") + val bytes = byteArrayOf(1, 2, 3, 4) + + val file = stagePastedImage(ByteArrayInputStream(bytes), dir, "png", maxBytes = 4) + + assertArrayEquals(bytes, file!!.readBytes()) + assertEquals(4L, file.length()) + assertTrue(file.name.endsWith(".png")) + assertEquals(listOf(file), dir.listFiles()!!.toList()) + } + + @Test + fun callerLimitAbovePerFileCapStillRejectsAfterTenMiBAndCleansUp() { + val dir = temp.newFolder("per-file-cap") + val input = ByteArrayInputStream(ByteArray(10 * 1024 * 1024 + 1)) + try { + stagePastedImage(input, dir, "png", maxBytes = 20L * 1024 * 1024) + fail("Expected the per-file limit") + } catch (_: PastedImageTooLargeException) { + assertTrue(dir.listFiles()!!.isEmpty()) + } + } + + @Test + fun unknownSizeIsBoundedByTheActualStreamAndReadsAtMostLimitPlusOne() { + val dir = temp.newFolder("oversize") + val input = + object : ByteArrayInputStream(ByteArray(100)) { + var consumed = 0 + + override fun read( + b: ByteArray, + off: Int, + len: Int, + ): Int = super.read(b, off, len).also { if (it > 0) consumed += it } + } + + try { + stagePastedImage(input, dir, "png", maxBytes = 4) + fail("Expected size limit rejection") + } catch (_: PastedImageTooLargeException) { + // The exact limit is accepted by the preceding test; only the extra byte rejects. + } + assertEquals(5, input.consumed) + assertTrue(dir.listFiles()!!.isEmpty()) + } + + @Test + fun emptyStreamIsRejectedAndLeavesNoCacheFile() { + val dir = temp.newFolder("empty") + + assertNull(stagePastedImage(ByteArrayInputStream(byteArrayOf()), dir, "webp", maxBytes = 4)) + assertTrue(dir.listFiles()!!.isEmpty()) + } + + @Test + fun streamFailureDeletesPartialFileAndClosesSource() { + val dir = temp.newFolder("broken") + val input = + object : InputStream() { + var reads = 0 + var closed = false + + override fun read(): Int = if (reads++ == 0) 7 else throw IOException("provider failed") + + override fun read( + b: ByteArray, + off: Int, + len: Int, + ): Int = + if (reads++ == 0) { + b[off] = 7 + 1 + } else { + throw IOException("provider failed") + } + + override fun close() { + closed = true + } + } + + try { + stagePastedImage(input, dir, "png", maxBytes = 4) + fail("Expected stream failure") + } catch (_: IOException) { + assertTrue(input.closed) + assertTrue(dir.listFiles()!!.isEmpty()) + } + } + + @Test + fun existingNameCollisionNeverTruncatesOrDeletesExistingFile() { + val dir = temp.newFolder("collision") + val id = UUID.randomUUID().toString() + val sentinel = dir.resolve("$id.png") + sentinel.writeText("keep me") + + try { + stagePastedImage(ByteArrayInputStream(byteArrayOf(1)), dir, "png", newId = { id }) + fail("Expected exclusive-create failure") + } catch (_: IOException) { + assertEquals("keep me", sentinel.readText()) + assertEquals(listOf(sentinel), dir.listFiles()!!.toList()) + } + } + + @Test + fun simultaneousImportsUseDistinctFilesAndKeepTheirOwnBytes() { + val dir = temp.newFolder("concurrent") + val executor = Executors.newFixedThreadPool(8) + try { + val tasks = + (1..16).map { n -> + Callable { + val bytes = ByteArray(n) { n.toByte() } + val file = stagePastedImage(ByteArrayInputStream(bytes), dir, "jpeg", maxBytes = 64)!! + file to bytes + } + } + val results = executor.invokeAll(tasks).map { it.get(10, TimeUnit.SECONDS) } + + assertEquals(16, results.map { it.first.name }.toSet().size) + results.forEach { (file, bytes) -> assertArrayEquals(bytes, file.readBytes()) } + assertEquals(16, dir.listFiles()!!.size) + } finally { + executor.shutdownNow() + } + } + + @Test + fun cancellationCheckDeletesPartialFileAndPropagatesCancellation() { + val dir = temp.newFolder("cancel") + var checks = 0 + val input = ByteArrayInputStream(ByteArray(100)) + + try { + stagePastedImage( + input, + dir, + "png", + maxBytes = 100, + checkActive = { + if (++checks == 2) throw CancellationException("cancelled") + }, + ) + fail("Expected cancellation") + } catch (_: CancellationException) { + assertTrue(dir.listFiles()!!.isEmpty()) + assertTrue(checks >= 2) + } + } + + @Test + fun aDiscardedResultDeletesOnlyItsOwnedFile() { + val dir = temp.newFolder("discard") + val file = stagePastedImage(ByteArrayInputStream(byteArrayOf(1)), dir, "png")!! + val unrelated = dir.resolve("unrelated.png").apply { writeText("keep") } + val result = + PastedImage( + attachment = + com.m57.hermescontrol.data.model.Attachment( + uri = "content://example/file", + name = file.name, + mimeType = "image/png", + size = 1, + ), + file = file, + ) + + result.discard() + assertFalse(file.exists()) + assertTrue(unrelated.exists()) + assertEquals("keep", unrelated.readText()) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSendStoreTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSendStoreTest.kt index 3dbac57f..79550d07 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSendStoreTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSendStoreTest.kt @@ -49,6 +49,29 @@ class ChatSendStoreTest { assertEquals(setOf("send-1"), confirmed) } + @Test + fun rowIdOnHistoryPageConfirmsAcceptedReceiptWithoutMergeMatch() { + val receipts = + listOf( + pending("send-1", "A").copy(userRowId = 334755L), + pending("send-2", "B").copy(userRowId = 999L), + pending("send-3", "C"), + ) + + val confirmed = pendingSendIdsConfirmedByRowIds(setOf(334755L), receipts) + + assertEquals(setOf("send-1"), confirmed) + } + + @Test + fun rowIdReceiptIsNotDowngradedButIdlessReceiptIs() { + val withRow = pending("send-1", "A").copy(userRowId = 334755L) + val idless = pending("send-2", "B") + + assertTrue(!canDemoteAcceptedReceipt(withRow)) + assertTrue(canDemoteAcceptedReceipt(idless)) + } + @Test fun durableAliasConsumesOnlyOneDuplicateReceiptOccurrence() { val existing = diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt index e2956a37..fae8ca74 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt @@ -360,6 +360,14 @@ class ChatViewModelTest { arg<((String) -> Unit)?>(2)?.invoke(id) id } + // Typed redirects bypass send(); keep this fixture off the real singleton transport. + every { HermesWsClient.sendRedirect(any(), any(), any()) } answers { + reqCount++ + val id = "req-redirect-$reqCount" + sentRequestMethods += WsMethods.SESSION_REDIRECT to id + arg<((String) -> Unit)?>(2)?.invoke(id) + id + } every { HermesWsClient.respondToServerRequest(any(), any()) } returns true every { HermesWsClient.respondToServerRequestError(any(), any(), any()) } returns true @@ -396,6 +404,73 @@ class ChatViewModelTest { unmockkAll() } + @Test + fun pastedImages_areAddedOnlyToCapturedReadySessionWithoutSubmitting() = + runTest(testDispatcher) { + val (viewModel, _) = createViewModelWithSession() + val target = checkNotNull(viewModel.captureAttachmentTarget()) + val image = Attachment("content://test/pasted.png", "pasted.png", "image/png", 4) + val messagesBefore = viewModel.uiState.value.messages + assertTrue(viewModel.addPastedAttachments(target, listOf(image))) + runCurrent() + assertEquals(listOf(image), viewModel.uiState.value.pendingAttachments) + assertEquals(messagesBefore, viewModel.uiState.value.messages) + verify(exactly = 0) { HermesWsClient.sendMessage(any(), any(), any(), any()) } + } + + @Test + fun pastedImages_rejectLateCompletionAfterSwitchAwayAndBack() = + runTest(testDispatcher) { + val (viewModel, sessionId) = createViewModelWithSession() + val target = checkNotNull(viewModel.captureAttachmentTarget()) + viewModel.switchSession("other-paste-test-session") + viewModel.switchSession(sessionId) + val image = Attachment("content://test/pasted.png", "pasted.png", "image/png", 4) + assertFalse(viewModel.addPastedAttachments(target, listOf(image))) + assertTrue( + viewModel.uiState.value.pendingAttachments + .isEmpty(), + ) + } + + @Test + fun pastedImages_rejectChangedServerAndProfilesAndGeneration() = + runTest(testDispatcher) { + val (viewModel, _) = createViewModelWithSession() + val target = checkNotNull(viewModel.captureAttachmentTarget()) + val image = Attachment("content://test/pasted.png", "pasted.png", "image/png", 4) + for (stale in listOf( + target.copy(baseUrl = "http://other-paste-test.local/"), + target.copy(connectionProfileId = "other-connection"), + target.copy(agentProfileId = "other-agent"), + target.copy(generation = target.generation + 1), + )) { + assertFalse(viewModel.addPastedAttachments(stale, listOf(image))) + } + assertTrue( + viewModel.uiState.value.pendingAttachments + .isEmpty(), + ) + } + + @Test + fun pastedImages_cannotCaptureTargetBeforeReady() = + runTest(testDispatcher) { + val viewModel = createViewModel() + advanceUntilIdle() + assertNull(viewModel.captureAttachmentTarget()) + } + + @Test + fun pastedImages_rejectDisconnectedSessionBeforeCombinedUiStateCatchesUp() = + runTest(testDispatcher) { + val (viewModel, _) = createViewModelWithSession() + val target = checkNotNull(viewModel.captureAttachmentTarget()) + mockConnectionStatus.value = ConnectionStatus.DISCONNECTED + assertNull(viewModel.captureAttachmentTarget()) + assertFalse(viewModel.addPastedAttachments(target, emptyList())) + } + // ── Helpers ────────────────────────────────────────────────────────────── /** Create a ViewModel with the fake repo injected directly. */ @@ -498,6 +573,32 @@ class ChatViewModelTest { assertEquals("op-1218", operation?.opId) } + @Test + fun sessionResume_sendsDesktopSourceLikeSessionCreate() = + runTest { + // #1450: session.create declared source="desktop" but session.resume omitted it, so + // the gateway resolved the resumed runtime from its host env ("tui") and staged a + // bogus surface switch that also unloaded the desktop_ui toolset. + val viewModel = createViewModel() + advanceUntilIdle() + + val captured = mutableListOf>>() + every { HermesWsClient.send(any(), any(), any()) } answers { + val id = "req-${captured.size + 1}" + captured.add(arg(0) to (arg>(1))) + arg<((String) -> Unit)?>(2)?.invoke(id) + id + } + + viewModel.switchSession("stored-session") + advanceUntilIdle() + + val resumeSent = captured.firstOrNull { it.first == WsMethods.SESSION_RESUME } + assertNotNull("session.resume should be dispatched", resumeSent) + assertEquals(JsonPrimitive("stored-session"), resumeSent!!.second["session_id"]) + assertEquals(JsonPrimitive("desktop"), resumeSent.second["source"]) + } + @Test fun sessionResume_restoresRetainedFailureAsNonDurablePartial() = runTest { @@ -627,7 +728,7 @@ class ChatViewModelTest { val (viewModel, sessionId) = createViewModelWithSession() every { - HermesWsClient.request(WsMethods.COMMAND_DISPATCH, any(), any()) + HermesWsClient.request(WsMethods.SLASH_EXEC, any(), any(), true) } returns CompletableDeferred( mapOf("type" to "exec", "output" to "**Available Commands:**\n\u2022 `/status`\n\u2022 `/new`"), @@ -1287,7 +1388,7 @@ class ChatViewModelTest { viewModel.sendMessage("/status") advanceUntilIdle() - verify { HermesWsClient.send(WsMethods.COMMAND_DISPATCH, any(), any()) } + verify { HermesWsClient.request(WsMethods.SLASH_EXEC, any(), any(), true) } } @Test @@ -1298,7 +1399,7 @@ class ChatViewModelTest { viewModel.sendMessage("/sessions") advanceUntilIdle() - verify { HermesWsClient.send(WsMethods.COMMAND_DISPATCH, any(), any()) } + verify { HermesWsClient.request(WsMethods.SLASH_EXEC, any(), any(), true) } } @Test @@ -1309,7 +1410,7 @@ class ChatViewModelTest { viewModel.sendMessage("/stats") advanceUntilIdle() - verify { HermesWsClient.send(WsMethods.COMMAND_DISPATCH, any(), any()) } + verify { HermesWsClient.request(WsMethods.SLASH_EXEC, any(), any(), true) } } @Test @@ -1635,6 +1736,7 @@ class ChatViewModelTest { JsonObject( mapOf( "session_id" to JsonPrimitive("session-from-notification"), + "source" to JsonPrimitive("desktop"), "omit_messages" to JsonPrimitive(true), ), ), @@ -5088,6 +5190,7 @@ class ChatViewModelTest { JsonObject( mapOf( "session_id" to JsonPrimitive("session-456"), + "source" to JsonPrimitive("desktop"), "omit_messages" to JsonPrimitive(true), ), ), @@ -5943,6 +6046,7 @@ class ChatViewModelTest { JsonObject( mapOf( "session_id" to JsonPrimitive("session-456"), + "source" to JsonPrimitive("desktop"), "omit_messages" to JsonPrimitive(true), ), ), @@ -10636,7 +10740,8 @@ class ChatViewModelTest { .last() .content, ) - assertEquals(PendingSendState.UNKNOWN, store.all().single().state) + // #1427: a mismatched history row cannot retire the gateway-confirmed receipt. + assertEquals(PendingSendState.ACCEPTED, store.all().single().state) assertTrue(vm.uiState.value.isSessionReady) assertTrue(vm.sendMessage("probe-second")) advanceUntilIdle() @@ -10648,9 +10753,10 @@ class ChatViewModelTest { assertNull(vm.uiState.value.errorMessage) verify(exactly = 1) { HermesWsClient.sendMessage(session, "probe-second", any(), any()) } verify(exactly = 1) { HermesWsClient.sendMessage(session, "probe-first", any(), any()) } - assertEquals(PendingSendState.UNKNOWN, store.all().single { it.text == "probe-first" }.state) + assertEquals(PendingSendState.ACCEPTED, store.all().single { it.text == "probe-first" }.state) + // The accepted row 10 remains visible separately from the unrelated history row 20. assertEquals( - 1, + 2, vm.transcriptState.value.messages .count { it.content == "probe-first" }, ) diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt index 1b2ae77b..de86c33b 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt @@ -478,4 +478,27 @@ class FullBleedTurnsTest { assertEquals(setOf("u1", "a2"), matchedMessageIds(msgs, listOf(0, 2, 0))) assertEquals(emptySet(), matchedMessageIds(msgs, listOf(9))) } + + @Test + fun `react_to_message tool rows are hidden but other tools stay`() { + val messages = + listOf( + msg("u1", MessageRole.USER), + msg("react", MessageRole.TOOL).copy(toolName = "react_to_message"), + msg("term", MessageRole.TOOL).copy(toolName = "terminal"), + msg("a1", MessageRole.ASSISTANT), + ) + val agent = groupIntoTurns(messages).filterIsInstance().single() + assertEquals( + listOf("term", "a1"), + agent.entries.map { + when (it) { + is AgentEntry.Prose -> it.message.id + is AgentEntry.ToolRow -> it.message.id + is AgentEntry.SystemEvent -> it.message.id + } + }, + ) + assertEquals(emptyList(), ChatSearchController().findMatches(messages.take(2), "react").matches) + } } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/GatewayNoticeTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/GatewayNoticeTest.kt new file mode 100644 index 00000000..1a798648 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/GatewayNoticeTest.kt @@ -0,0 +1,233 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.local.toEntity +import com.m57.hermescontrol.data.local.toUiModel +import com.m57.hermescontrol.data.model.SessionMessage +import com.m57.hermescontrol.data.ws.WsEvent +import kotlinx.serialization.json.JsonPrimitive +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** #1433: gateway notices are per-session transcript rows that keep their position. */ +class GatewayNoticeTest { + private val compactedText = "✓ Context compaction complete — continuing turn..." + + private fun status( + kind: String, + text: String?, + sessionId: String? = "s1", + ) = WsEvent.StatusUpdate( + status = null, + data = mapOf("kind" to kind, "text" to text), + sessionId = sessionId, + ) + + private fun reduce( + state: ChatUiState, + event: WsEvent, + streaming: StreamingState = StreamingState(), + ) = ChatWsEventReducer.reduce(state, streaming, event, currentSessionId = "s1") + + @Test + fun sentinelMatchesEveryGatewayInterruptShape() { + listOf( + "[the reply degenerated into a repetition loop and was interrupted]", + "Operation interrupted.", + "Operation interrupted: waiting for the provider to recover (cycle 1/3).", + "Operation interrupted: retrying API call after error (retry 2/5).", + "Operation interrupted: handling API error (RateLimitError: slow down).", + "Operation interrupted: retrying empty response from model (retry 1/2).", + "Operation interrupted during retry (timeout, attempt 1/3).", + " Operation interrupted.\n", + ).forEach { assertTrue(it, GatewayNotice.isInterruptSentinel(it)) } + listOf( + "Operation interrupted", + "Operation interrupted. Here is what I found so far", + "The operation was interrupted.", + "Operation interrupted: first line.\nsecond line.", + ).forEach { assertFalse(it, GatewayNotice.isInterruptSentinel(it)) } + } + + @Test + fun compactedNoticeBecomesPersistedSystemRowAndClearsTailState() { + val user = ChatMessage(id = "u", role = MessageRole.USER, content = "go") + val state = + ChatUiState( + currentSessionId = "s1", + messages = listOf(user), + isCompressing = true, + compressionStatus = "🗜️ Compacting context", + ) + + val result = reduce(state, status("compacted", compactedText)) + + assertFalse(result.state.isCompressing) + assertNull(result.state.compressionStatus) + val notice = result.state.messages.last() + assertEquals(MessageRole.SYSTEM, notice.role) + assertEquals(compactedText, notice.content) + assertTrue(result.effects.any { it is ReducerEffect.PersistMessage && it.message.id == notice.id }) + } + + @Test + fun replayedNoticeInSameTurnDoesNotStack() { + val state = + ChatUiState( + currentSessionId = "s1", + messages = listOf(ChatMessage(role = MessageRole.USER, content = "go")), + ) + val once = reduce(state, status("compacted", compactedText)).state + val twice = reduce(once, status("compacted", compactedText)).state + assertEquals(1, twice.messages.count { it.role == MessageRole.SYSTEM }) + + val nextTurn = twice.copy(messages = twice.messages + ChatMessage(role = MessageRole.USER, content = "again")) + val again = reduce(nextTurn, status("compacted", compactedText)).state + assertEquals(2, again.messages.count { it.role == MessageRole.SYSTEM }) + } + + @Test + fun otherSessionsStatusUpdatesAreIgnored() { + val state = ChatUiState(currentSessionId = "s1") + listOf("compacting", "compacted", "goal", "warn").forEach { kind -> + val result = reduce(state, status(kind, "text for $kind", sessionId = "other")) + assertEquals(kind, state, result.state) + } + } + + @Test + fun durableKindsAppendAndTransientKindsDoNot() { + val state = ChatUiState(currentSessionId = "s1") + listOf("goal", "loop", "warn", "fallback").forEach { kind -> + assertEquals(kind, 1, reduce(state, status(kind, "✓ $kind")).state.messages.size) + } + listOf("status", "lifecycle", "process", "heartbeat", "ready").forEach { kind -> + assertTrue(kind, reduce(state, status(kind, "x")).state.messages.isEmpty()) + } + } + + @Test + fun repetitionLoopCompletionDropsLoopedPartialAndAppendsNotice() { + val sealed = ChatMessage(id = "looped", role = MessageRole.ASSISTANT, content = "again again again") + val state = ChatUiState(currentSessionId = "s1", messages = listOf(sealed), isAgentTyping = true) + val event = + WsEvent.MessageComplete( + text = GatewayNotice.REPETITION_LOOP_INTERRUPTED, + sessionId = "s1", + rawPayload = mapOf("status" to "interrupted"), + ) + + val result = reduce(state, event, StreamingState(interruptedMessage = sealed)) + + assertEquals(listOf(MessageRole.SYSTEM), result.state.messages.map { it.role }) + assertEquals( + GatewayNotice.REPETITION_LOOP_INTERRUPTED, + result.state.messages + .single() + .content, + ) + assertFalse(result.state.isAgentTyping) + assertTrue(result.effects.contains(ReducerEffect.DeleteLocalMessage("looped"))) + } + + @Test + fun operationInterruptedCompletionKeepsRealPartial() { + val stream = ChatMessage(id = "p", role = MessageRole.ASSISTANT, content = "half an answer", isStreaming = true) + val state = ChatUiState(currentSessionId = "s1") + val event = WsEvent.MessageComplete(text = "Operation interrupted.", sessionId = "s1") + + val result = reduce(state, event, StreamingState(streamingMessage = stream)) + + assertEquals( + listOf("p", null), + result.state.messages.map { + if (it.role == + MessageRole.SYSTEM + ) { + null + } else { + it.id + } + }, + ) + assertFalse( + result.state.messages + .first() + .isStreaming, + ) + assertEquals( + "Operation interrupted.", + result.state.messages + .last() + .content, + ) + } + + @Test + fun persistedSentinelRowMapsToSystemNoticeAndMergesWithLiveCopy() { + val rows = + listOf( + SessionMessage(id = 1, role = "user", content = JsonPrimitive("go")), + SessionMessage(id = 2, role = "assistant", content = JsonPrimitive("Operation interrupted.")), + SessionMessage(id = 3, role = "user", content = JsonPrimitive("next")), + SessionMessage(id = 4, role = "assistant", content = JsonPrimitive("done")), + ) + val page = mapServerMessages("s", rows, 0, true, emptyList()) + assertEquals(MessageRole.SYSTEM, page[1].role) + + // Live view after the interrupted turn: the notice sits between the two turns. + val live = + listOf( + ChatMessage(id = "u1", role = MessageRole.USER, content = "go"), + ChatMessage(id = "n", role = MessageRole.SYSTEM, content = "Operation interrupted."), + ChatMessage(id = "u2", role = MessageRole.USER, content = "next"), + ChatMessage(id = "a2", role = MessageRole.ASSISTANT, content = "done"), + ) + val merged = mergeTranscriptWithLive(page, live, preserveLiveIds = true) + assertEquals( + listOf("go", "Operation interrupted.", "next", "done"), + merged.map { it.content }, + ) + } + + @Test + fun liveOnlyNoticeStaysInPlaceAcrossLaterTurnsAndRefresh() { + val page = + mapServerMessages( + "s", + listOf( + SessionMessage(id = 1, role = "user", content = JsonPrimitive("first")), + SessionMessage(id = 2, role = "assistant", content = JsonPrimitive("one")), + SessionMessage(id = 3, role = "user", content = JsonPrimitive("second")), + SessionMessage(id = 4, role = "assistant", content = JsonPrimitive("two")), + ), + 0, + true, + emptyList(), + ) + val live = + listOf( + ChatMessage(id = "u1", role = MessageRole.USER, content = "first"), + ChatMessage(id = "a1", role = MessageRole.ASSISTANT, content = "one"), + ChatMessage(id = "c", role = MessageRole.SYSTEM, content = compactedText), + ChatMessage(id = "u2", role = MessageRole.USER, content = "second"), + ChatMessage(id = "a2", role = MessageRole.ASSISTANT, content = "two"), + ) + val once = mergeTranscriptWithLive(page, live, preserveLiveIds = true) + val twice = mergeTranscriptWithLive(page, once, preserveLiveIds = true) + val expected = listOf("first", "one", compactedText, "second", "two") + assertEquals(expected, once.map { it.content }) + assertEquals(expected, twice.map { it.content }) + } + + @Test + fun roomRoundTripNormalizesLegacyAssistantSentinel() { + val legacy = ChatMessage(id = "x", role = MessageRole.ASSISTANT, content = "Operation interrupted.") + val restored = legacy.toEntity("s").toUiModel() + assertEquals(MessageRole.SYSTEM, restored.role) + val prose = ChatMessage(id = "y", role = MessageRole.ASSISTANT, content = "fine") + assertEquals(MessageRole.ASSISTANT, prose.toEntity("s").toUiModel().role) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt index 029b9311..1a7c529a 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt @@ -161,7 +161,7 @@ class SlashCommandDispatchRpcTest { // can end up holding session.usage instead of command.dispatch (CI run // 31137581335 on 4645a2b: slash-focus flake). // Specific per-test request() stubs registered later take precedence. - every { HermesWsClient.request(any(), any(), any()) } answers { + every { HermesWsClient.request(any(), any(), any(), any()) } answers { HermesWsClient.send(arg(0), arg(1)) {} CompletableDeferred(Unit) } @@ -194,7 +194,7 @@ class SlashCommandDispatchRpcTest { } @Test - fun `non-hardcoded slash command forwards name arg session_id to COMMAND_DISPATCH`() = + fun `non-hardcoded slash command forwards command and session_id to SLASH_EXEC`() = runTest { val (vm, sessionId) = createViewModelWithSession() @@ -202,7 +202,7 @@ class SlashCommandDispatchRpcTest { val paramsCalls = mutableListOf>() var captured = false every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any(), any()) } answers { captured = true CompletableDeferred(Unit) @@ -216,14 +216,67 @@ class SlashCommandDispatchRpcTest { // Captures hold ALL requests — the create flow's session.usage / // context_breakdown can land after the dispatch one (capture race, // CI 2026-08-06 + 2026-08-07). Find the record, don't trust "last". - val dispatchIndex = methodCalls.indexOf(WsMethods.COMMAND_DISPATCH) - assertTrue("expected COMMAND_DISPATCH, got $methodCalls", dispatchIndex >= 0) - val params = paramsCalls[dispatchIndex] - assertEquals(JsonPrimitive("help"), params["name"]) - assertEquals(JsonPrimitive(""), params["arg"]) + val execIndex = methodCalls.indexOf(WsMethods.SLASH_EXEC) + assertTrue("expected SLASH_EXEC, got $methodCalls", execIndex >= 0) + assertTrue(WsMethods.COMMAND_DISPATCH !in methodCalls) + val params = paramsCalls[execIndex] + assertEquals(JsonPrimitive("help"), params["command"]) assertEquals(JsonPrimitive(sessionId), params["session_id"]) } + @Test + fun `usage renders warning and suppresses intermediate error events`() = + runTest { + val (vm, _) = createViewModelWithSession() + every { HermesWsClient.request(WsMethods.SLASH_EXEC, any(), any(), true) } returns + CompletableDeferred(mapOf("output" to "Session Token Usage", "warning" to "estimated")) + + vm.sendMessage("/usage") + advanceUntilIdle() + + verify(exactly = 1) { HermesWsClient.request(WsMethods.SLASH_EXEC, any(), any(), true) } + verify(exactly = 0) { HermesWsClient.request(WsMethods.COMMAND_DISPATCH, any(), any(), any()) } + assertEquals( + "warning: estimated\nSession Token Usage", + vm.uiState.value.messages + .last() + .content, + ) + assertNull(vm.uiState.value.errorMessage) + } + + @Test + fun `slash failure falls back with name arg and suppresses both error events`() = + runTest { + val (vm, sessionId) = createViewModelWithSession() + val calls = mutableListOf() + val params = mutableListOf>() + every { HermesWsClient.request(capture(calls), capture(params), any(), true) } answers { + if (arg(0) == WsMethods.SLASH_EXEC) { + CompletableDeferred().apply { + completeExceptionally(HermesWsClient.HermesRpcException("worker unavailable")) + } + } else { + CompletableDeferred(mapOf("type" to "plugin", "output" to "plugin result")) + } + } + + vm.sendMessage("/custom some args") + advanceUntilIdle() + + assertEquals(listOf(WsMethods.SLASH_EXEC, WsMethods.COMMAND_DISPATCH), calls) + assertEquals(JsonPrimitive("custom"), params[1]["name"]) + assertEquals(JsonPrimitive("some args"), params[1]["arg"]) + assertEquals(JsonPrimitive(sessionId), params[1]["session_id"]) + assertEquals( + "plugin result", + vm.uiState.value.messages + .last() + .content, + ) + assertNull(vm.uiState.value.errorMessage) + } + @Test fun `slash queue submits the arg with queued=true instead of command dispatch`() = runTest { @@ -235,9 +288,15 @@ class SlashCommandDispatchRpcTest { val sentText = slot() val sentQueued = slot() every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any(), any()) } answers { - CompletableDeferred(Unit) + if (arg(0) == WsMethods.SLASH_EXEC) { + CompletableDeferred().apply { + completeExceptionally(HermesWsClient.HermesRpcException("slash unavailable")) + } + } else { + CompletableDeferred(Unit) + } } every { HermesWsClient.sendMessage( @@ -372,9 +431,15 @@ class SlashCommandDispatchRpcTest { val rpcMethods = mutableListOf() var sendCalls = 0 every { - HermesWsClient.request(capture(rpcMethods), any(), any()) + HermesWsClient.request(capture(rpcMethods), any(), any(), any()) } answers { - CompletableDeferred(Unit) + if (arg(0) == WsMethods.SLASH_EXEC) { + CompletableDeferred().apply { + completeExceptionally(HermesWsClient.HermesRpcException("slash unavailable")) + } + } else { + CompletableDeferred(Unit) + } } every { HermesWsClient.sendMessage(any(), any(), any(), any()) } answers { sendCalls++ @@ -416,12 +481,12 @@ class SlashCommandDispatchRpcTest { } @Test - fun `non-registry-miss backend error is surfaced directly (no fallback)`() = + fun `actionable fallback error is surfaced`() = runTest { val (vm, _) = createViewModelWithSession() every { - HermesWsClient.request(any(), any(), any()) + HermesWsClient.request(any(), any(), any(), any()) } answers { val d = CompletableDeferred() d.completeExceptionally( @@ -442,14 +507,14 @@ class SlashCommandDispatchRpcTest { } @Test - fun `registry-miss 4018 on command dispatch falls back to slash_exec and surfaces output`() = + fun `slash exec success surfaces output without command dispatch`() = runTest { val (vm, sessionId) = createViewModelWithSession() val methodCalls = mutableListOf() val paramsCalls = mutableListOf>() every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any(), any()) } answers { // Read the method from THIS invocation, not from the shared // capture list: a concurrent request() landing between capture @@ -474,16 +539,14 @@ class SlashCommandDispatchRpcTest { vm.sendMessage("/status") advanceUntilIdle() - // The fallback must have hit slash.exec with the full command string. + // Successful slash.exec must not need a fallback. // Captures hold ALL requests — find the records instead of trusting // "last" (create-flow session.usage can land after; capture race). - val dispatchIndex = methodCalls.indexOf(WsMethods.COMMAND_DISPATCH) - assertTrue("expected COMMAND_DISPATCH, got $methodCalls", dispatchIndex >= 0) + assertTrue(WsMethods.COMMAND_DISPATCH !in methodCalls) val execIndex = methodCalls.indexOf(WsMethods.SLASH_EXEC) - assertTrue("expected SLASH_EXEC fallback, got $methodCalls", execIndex >= 0) - assertTrue("dispatch must precede fallback", dispatchIndex < execIndex) + assertTrue("expected SLASH_EXEC, got $methodCalls", execIndex >= 0) val params = paramsCalls[execIndex] - assertEquals(JsonPrimitive("/status"), params["command"]) + assertEquals(JsonPrimitive("status"), params["command"]) assertEquals(JsonPrimitive(sessionId), params["session_id"]) // And the slash.exec output must be surfaced to the user. @@ -500,9 +563,15 @@ class SlashCommandDispatchRpcTest { val rpcMethods = mutableListOf() every { - HermesWsClient.request(capture(rpcMethods), any(), any()) + HermesWsClient.request(capture(rpcMethods), any(), any(), any()) } answers { - CompletableDeferred(Unit) + if (arg(0) == WsMethods.SLASH_EXEC) { + CompletableDeferred().apply { + completeExceptionally(HermesWsClient.HermesRpcException("slash unavailable")) + } + } else { + CompletableDeferred(Unit) + } } // /redraw is TUI-only (issue #574) — hidden from suggestions but a @@ -533,12 +602,12 @@ class SlashCommandDispatchRpcTest { } @Test - fun `slash_exec double-fault surfaces the secondary error`() = + fun `fallback registry miss preserves original slash exec error`() = runTest { val (vm, _) = createViewModelWithSession() every { - HermesWsClient.request(any(), any(), any()) + HermesWsClient.request(any(), any(), any(), any()) } answers { val m = arg(0) val d = CompletableDeferred() @@ -561,7 +630,7 @@ class SlashCommandDispatchRpcTest { vm.sendMessage("/status") advanceUntilIdle() - // Both RPCs fail -> the secondary slash.exec error must surface. + // A registry miss must not mask the original worker failure. val last = vm.uiState.value.messages .lastOrNull() @@ -569,12 +638,12 @@ class SlashCommandDispatchRpcTest { } @Test - fun `slash_exec blank output appends no assistant message`() = + fun `slash_exec blank output reports no output like desktop`() = runTest { val (vm, _) = createViewModelWithSession() every { - HermesWsClient.request(any(), any(), any()) + HermesWsClient.request(any(), any(), any(), any()) } answers { val m = arg(0) val d = CompletableDeferred() @@ -595,12 +664,10 @@ class SlashCommandDispatchRpcTest { vm.sendMessage("/status") advanceUntilIdle() - // The user's "/status" message is added, but blank slash.exec output - // must NOT append an assistant bubble — so the last message is still - // the user's own command, and only one message was added. - assertEquals(before + 1, vm.uiState.value.messages.size) + // Desktop reports an explicit no-output notice rather than silently succeeding. + assertEquals(before + 2, vm.uiState.value.messages.size) assertEquals( - "/status", + "/status: no output", vm.uiState.value.messages .lastOrNull() ?.content, @@ -615,9 +682,15 @@ class SlashCommandDispatchRpcTest { val methodCalls = mutableListOf() val paramsCalls = mutableListOf>() every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any(), any()) } answers { - CompletableDeferred(Unit) + if (arg(0) == WsMethods.SLASH_EXEC) { + CompletableDeferred().apply { + completeExceptionally(HermesWsClient.HermesRpcException("slash unavailable")) + } + } else { + CompletableDeferred(Unit) + } } // /init generates-or-updates AGENTS.md (backend command.dispatch @@ -643,9 +716,15 @@ class SlashCommandDispatchRpcTest { val methodCalls = mutableListOf() val paramsCalls = mutableListOf>() every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any(), any()) } answers { - CompletableDeferred(Unit) + if (arg(0) == WsMethods.SLASH_EXEC) { + CompletableDeferred().apply { + completeExceptionally(HermesWsClient.HermesRpcException("slash unavailable")) + } + } else { + CompletableDeferred(Unit) + } } vm.sendMessage("/init extra context here") @@ -668,9 +747,15 @@ class SlashCommandDispatchRpcTest { val methodCalls = mutableListOf() val paramsCalls = mutableListOf>() every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any(), any()) } answers { - CompletableDeferred(Unit) + if (arg(0) == WsMethods.SLASH_EXEC) { + CompletableDeferred().apply { + completeExceptionally(HermesWsClient.HermesRpcException("slash unavailable")) + } + } else { + CompletableDeferred(Unit) + } } // /focus is the display-only focus view (backend command.dispatch @@ -696,9 +781,15 @@ class SlashCommandDispatchRpcTest { val methodCalls = mutableListOf() val paramsCalls = mutableListOf>() every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any(), any()) } answers { - CompletableDeferred(Unit) + if (arg(0) == WsMethods.SLASH_EXEC) { + CompletableDeferred().apply { + completeExceptionally(HermesWsClient.HermesRpcException("slash unavailable")) + } + } else { + CompletableDeferred(Unit) + } } vm.sendMessage("/focus on") @@ -714,7 +805,7 @@ class SlashCommandDispatchRpcTest { } @Test - fun `command dispatch type=send submits the returned prompt (init)`() = + fun `slash exec type=send submits the returned prompt (init)`() = runTest { val (vm, _) = createViewModelWithSession() @@ -722,12 +813,12 @@ class SlashCommandDispatchRpcTest { val paramsCalls = mutableListOf>() val sentText = slot() every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any(), any()) } answers { val d = CompletableDeferred() // Read the method from THIS invocation, not from the shared // capture list (see the 4018 test above). - if (arg(0) == WsMethods.COMMAND_DISPATCH) { + if (arg(0) == WsMethods.SLASH_EXEC) { // Backend /init returns type:"send" with the AGENTS.md prompt. d.complete(mapOf("type" to "send", "message" to "Scan this repo and write AGENTS.md")) } else { @@ -766,7 +857,7 @@ class SlashCommandDispatchRpcTest { val (vm, _) = createViewModelWithSession() every { - HermesWsClient.request(any(), any(), any()) + HermesWsClient.request(any(), any(), any(), any()) } answers { val m = arg(0) val d = CompletableDeferred() @@ -774,7 +865,7 @@ class SlashCommandDispatchRpcTest { // Backend /focus returns type:"exec" with a notice line. d.complete(mapOf("type" to "exec", "output" to "Focus view: ON (tool progress pinned off)")) } else { - d.complete(mapOf("session_id" to "session-xyz")) + d.completeExceptionally(HermesWsClient.HermesRpcException("slash unavailable")) } d } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/UserImageHydrationTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/UserImageHydrationTest.kt new file mode 100644 index 00000000..63862bbb --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/UserImageHydrationTest.kt @@ -0,0 +1,355 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.local.toEntity +import com.m57.hermescontrol.data.local.toUiModel +import com.m57.hermescontrol.data.model.AttachmentSource +import com.m57.hermescontrol.data.model.SessionMessage +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonArray +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Regression tests for user image hydration: + * - Direct unit tests for [userImageAttachments] + * - Integration via [mapServerMessages] with USER turns + */ +class UserImageHydrationTest { + @Test + fun restHydrationEnrichesCachedUserWithoutReplacingLocalAttachments() { + val rest = + mapServerMessages( + sessionId = "session-1", + messages = + listOf( + SessionMessage(id = 42, role = "user", content = JsonPrimitive("caption\n@image:/image.png")), + ), + offset = 0, + latestPaging = true, + liveMessages = emptyList(), + mediaUrl = { "https://gateway.test/image.png" }, + ).single() + val cached = + rest.copy( + attachments = null, + content = "[{\"image_url\":\"data:image/png;base64,old\"}]", + isHistoricalCache = true, + ) + val hydrated = mergeTranscriptWithLive(listOf(rest), listOf(cached)).single() + assertEquals(rest.attachments, hydrated.attachments) + assertEquals(rest.content, hydrated.content) + assertEquals(cached.id, hydrated.id) + val localAttachments = + listOf( + com.m57.hermescontrol.data.model + .Attachment("content://local/image", "image.png", "image/png"), + ) + val local = rest.copy(attachments = localAttachments) + assertEquals(localAttachments, mergeTranscriptWithLive(listOf(rest), listOf(local)).single().attachments) + } + + @Test + fun roomRoundTripKeepsReferencesForGatewayReconstruction() { + val content = "caption\n@image:/image.png" + val message = ChatMessage(id = "cached", role = MessageRole.USER, content = content) + val restored = message.toEntity("session-1").toUiModel() + assertEquals(content, restored.content) + val attachments = userImageAttachments(restored.content) { "https://current-gateway.test$it" } + assertEquals("https://current-gateway.test/image.png", attachments.single().gatewayUrl) + } + + @Test + fun userImageAttachments_recognizesTrimmedWholeLineImageDirectiveWithAbsolutePaths() { + val content = + """ + Here is my photo + @image:/absolute/path/to/cat.png + And another one + @image:/absolute/path/to/dog.jpg + """.trimIndent() + + val attachments = + userImageAttachments(content) { path -> "https://gateway.local/api/files/download?path=$path" } + + assertEquals(2, attachments.size) + + val first = attachments[0] + assertEquals("https://gateway.local/api/files/download?path=/absolute/path/to/cat.png", first.uri) + assertEquals("cat.png", first.name) + assertEquals("image/png", first.mimeType) + assertEquals(0L, first.size) + assertEquals("https://gateway.local/api/files/download?path=/absolute/path/to/cat.png", first.gatewayUrl) + assertEquals(AttachmentSource.GATEWAY, first.source) + assertTrue(first.isImage) + + val second = attachments[1] + assertEquals("https://gateway.local/api/files/download?path=/absolute/path/to/dog.jpg", second.uri) + assertEquals("dog.jpg", second.name) + assertEquals("image/jpeg", second.mimeType) + assertEquals(AttachmentSource.GATEWAY, second.source) + assertTrue(second.isImage) + } + + @Test + fun userImageAttachments_allowsSpacesInAbsolutePaths() { + val content = "@image:/Users/test user/My Documents/holiday photo 2026.png" + + val attachments = + userImageAttachments(content) { path -> "https://gateway.local/api/files/download?path=$path" } + + assertEquals(1, attachments.size) + val attachment = attachments.single() + assertEquals( + "https://gateway.local/api/files/download?path=/Users/test user/My Documents/holiday photo 2026.png", + attachment.uri, + ) + assertEquals("holiday photo 2026.png", attachment.name) + assertEquals("image/png", attachment.mimeType) + assertEquals(AttachmentSource.GATEWAY, attachment.source) + } + + @Test + fun userImageAttachments_deduplicatesRepeatPathsKeepingFirst() { + val content = + """ + First mention + @image:/images/duplicated.png + Second mention + @image:/images/duplicated.png + Third mention with whitespace padding + @image:/images/duplicated.png + Different image + @image:/images/unique.png + """.trimIndent() + + val attachments = userImageAttachments(content) { path -> "https://gateway.local/file?p=$path" } + + assertEquals(2, attachments.size) + assertEquals("https://gateway.local/file?p=/images/duplicated.png", attachments[0].uri) + assertEquals("duplicated.png", attachments[0].name) + assertEquals("https://gateway.local/file?p=/images/unique.png", attachments[1].uri) + assertEquals("unique.png", attachments[1].name) + } + + @Test + fun userImageAttachments_ignoresInlineProseEmptyAndRelativeReferences() { + val content = + """ + Do not match inline prose such as check out @image:/absolute/path/foo.png in middle of text + Do not match trailing inline text @image:/path/bar.png extra + Do not match relative path @image:relative/path/image.png + @image: + @image: + @image:relative/foo.jpg + @image:./local/foo.png + @image:../parent/foo.png + @image:not-an-absolute-path.jpg + Valid absolute path at start of line: + @image:/valid/match.png + """.trimIndent() + + val attachments = userImageAttachments(content) { path -> "http://gateway.test$path" } + + assertEquals(1, attachments.size) + assertEquals("http://gateway.test/valid/match.png", attachments.single().uri) + assertEquals("match.png", attachments.single().name) + } + + @Test + fun userImageAttachments_whenCallbackReturnsNull_returnsNoAttachments() { + val content = + """ + @image:/valid/photo1.jpg + @image:/valid/photo2.png + """.trimIndent() + + val attachments = userImageAttachments(content) { null } + + assertTrue(attachments.isEmpty()) + } + + @Test + fun userImageAttachments_whenContentHasNoImageDirectives_returnsEmptyList() { + val content = "Just a regular text conversation with no images attached." + + val attachments = userImageAttachments(content) { path -> "https://gw/$path" } + + assertTrue(attachments.isEmpty()) + } + + @Test + fun mapServerMessages_withUserTurn_attachesGatewayImageFromPlainStringContent() { + val userRow = + SessionMessage( + id = 1, + role = "user", + content = JsonPrimitive("Please review this design\n@image:/data/mockups/home.png"), + ) + + val mapped = + mapServerMessages( + sessionId = "session-1", + messages = listOf(userRow), + offset = 0, + latestPaging = true, + liveMessages = emptyList(), + mediaUrl = { path -> "https://gateway.local/files?p=$path" }, + ) + + assertEquals(1, mapped.size) + val chatMessage = mapped.single() + assertEquals(MessageRole.USER, chatMessage.role) + assertTrue(chatMessage.content.contains("Please review this design")) + assertTrue(chatMessage.content.contains("@image:/data/mockups/home.png")) + + val attachments = chatMessage.attachments + assertNotNull(attachments) + assertEquals(1, attachments!!.size) + + val att = attachments.single() + assertEquals("https://gateway.local/files?p=/data/mockups/home.png", att.uri) + assertEquals("home.png", att.name) + assertEquals("image/png", att.mimeType) + assertEquals(AttachmentSource.GATEWAY, att.source) + assertTrue(att.isGateway) + } + + @Test + fun mapServerMessages_withUserMultipart748kPayload_preservesCaptionAndNoBase64InContent() { + val largeBase64 = "data:image/png;base64," + "iVBORw0KGgoAAAANSUhEUgAA".repeat(31_000) // ~748k chars + val caption = "Check out this screenshot\n@image:/opt/data/images/screen.png" + + val multipart = + buildJsonArray { + add( + buildJsonObject { + put("type", "image_url") + putJsonObject("image_url") { + put("url", largeBase64) + } + }, + ) + add( + buildJsonObject { + put("type", "text") + put("text", caption) + }, + ) + } + + val userRow = + SessionMessage( + id = 42, + role = "user", + content = multipart, + ) + + val mapped = + mapServerMessages( + sessionId = "session-1", + messages = listOf(userRow), + offset = 0, + latestPaging = true, + liveMessages = emptyList(), + mediaUrl = { path -> "https://gw.internal/download?path=$path" }, + ) + + assertEquals(1, mapped.size) + val chatMessage = mapped.single() + + // Content must preserve the caption and reference text, but contain no base64 payload + assertTrue(chatMessage.content.contains("Check out this screenshot")) + assertTrue(chatMessage.content.contains("@image:/opt/data/images/screen.png")) + assertFalse(chatMessage.content.contains("data:image/png;base64")) + assertFalse(chatMessage.content.contains("iVBORw0KGgoAAAANSUhEUgAA")) + + // Attachment hydrated from @image directive + val attachments = chatMessage.attachments + assertNotNull(attachments) + assertEquals(1, attachments!!.size) + + val att = attachments.single() + assertEquals("https://gw.internal/download?path=/opt/data/images/screen.png", att.uri) + assertEquals("screen.png", att.name) + assertEquals("image/png", att.mimeType) + assertEquals(AttachmentSource.GATEWAY, att.source) + } + + @Test + fun mapServerMessages_withUserTurn_deduplicatesRepeatImagePaths() { + val content = + """ + Examining these photos: + @image:/data/photos/target.jpg + @image:/data/photos/target.jpg + @image:/data/photos/target.jpg + @image:/data/photos/another.png + """.trimIndent() + + val userRow = + SessionMessage( + id = 5, + role = "user", + content = JsonPrimitive(content), + ) + + val mapped = + mapServerMessages( + sessionId = "session-1", + messages = listOf(userRow), + offset = 0, + latestPaging = true, + liveMessages = emptyList(), + mediaUrl = { path -> "https://gw.internal$path" }, + ) + + val chatMessage = mapped.single() + val attachments = chatMessage.attachments + assertNotNull(attachments) + assertEquals(2, attachments!!.size) + assertEquals("https://gw.internal/data/photos/target.jpg", attachments[0].uri) + assertEquals("target.jpg", attachments[0].name) + assertEquals("https://gw.internal/data/photos/another.png", attachments[1].uri) + assertEquals("another.png", attachments[1].name) + } + + @Test + fun mapServerMessages_withUserTurn_whenGatewayUnavailable_preservesCaptionAndAddsNoAttachments() { + val caption = "Look at this error log\n@image:/var/log/screenshot.png" + val userRow = + SessionMessage( + id = 10, + role = "user", + content = JsonPrimitive(caption), + ) + + val mapped = + mapServerMessages( + sessionId = "session-1", + messages = listOf(userRow), + offset = 0, + latestPaging = true, + liveMessages = emptyList(), + mediaUrl = { null }, // Gateway unavailable + ) + + val chatMessage = mapped.single() + assertTrue(chatMessage.content.contains("Look at this error log")) + assertTrue(chatMessage.content.contains("@image:/var/log/screenshot.png")) + assertNull(chatMessage.attachments) + } + + @Test + fun hideImageRefLines_removesOnlyPathLinesAndKeepsCaption() { + val content = "caption\n@image:/opt/data/images/a.jpg\nmore text @image:/inline.png" + assertEquals("caption\nmore text @image:/inline.png", hideImageRefLines(content)) + assertEquals("plain", hideImageRefLines("plain")) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/components/ChatImagePasteControllerTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/components/ChatImagePasteControllerTest.kt new file mode 100644 index 00000000..3fddc533 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/components/ChatImagePasteControllerTest.kt @@ -0,0 +1,342 @@ +package com.m57.hermescontrol.ui.chat.components + +import android.net.Uri +import androidx.compose.foundation.ExperimentalFoundationApi +import com.m57.hermescontrol.R +import com.m57.hermescontrol.data.model.Attachment +import com.m57.hermescontrol.ui.chat.ChatAttachmentTarget +import com.m57.hermescontrol.ui.chat.PastedImage +import com.m57.hermescontrol.ui.chat.PastedImageTooLargeException +import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.job +import kotlinx.coroutines.joinAll +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.RandomAccessFile + +@OptIn(ExperimentalFoundationApi::class, ExperimentalCoroutinesApi::class) +class ChatImagePasteControllerTest { + @get:Rule val temp = TemporaryFolder() + private val target = ChatAttachmentTarget("session", 1, "http://test.local", null, null) + + private fun image(name: String): PastedImage { + val file = temp.newFile(name).apply { writeBytes(byteArrayOf(1, 2)) } + return PastedImage(Attachment("content://test/$name", name, "image/png", file.length()), file) + } + + @Test + fun successPublishesAnOrderedBatchOnlyAfterEveryImageIsCopied() = + runTest { + val first = image("one.png") + val second = image("two.png") + val gate = CompletableDeferred() + val committed = mutableListOf() + val errors = mutableListOf() + var count = 0 + val controller = + ChatImagePasteController( + this, + importImage = { _, _ -> + if (count++ == 0) { + first + } else { + gate.await() + second + } + }, + captureTarget = { target }, + commit = { _, attachments -> + committed.addAll(attachments) + true + }, + onError = errors::add, + ) + assertTrue(controller.receiveImages(listOf(mockk(), mockk()), Any())) + assertTrue(controller.isReceiving) + runCurrent() + assertTrue(committed.isEmpty()) + gate.complete(Unit) + coroutineContext.job.children + .toList() + .joinAll() + assertFalse(controller.isReceiving) + assertEquals(listOf(first.attachment, second.attachment), committed) + assertTrue(first.file.exists() && second.file.exists()) + assertTrue(errors.isEmpty()) + } + + @Test + fun laterUnreadableImageRollsBackTheEntireBatch() = + runTest { + val first = image("rollback.png") + var count = 0 + var commits = 0 + val errors = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, _ -> if (count++ == 0) first else null }, + captureTarget = { target }, + commit = { _, _ -> + commits++ + true + }, + onError = errors::add, + ) + controller.receiveImages(listOf(mockk(), mockk()), Any()) + coroutineContext.job.children + .toList() + .joinAll() + assertEquals(0, commits) + assertFalse(first.file.exists()) + assertEquals(listOf(R.string.chat_image_paste_failed), errors) + assertFalse(controller.isReceiving) + } + + @Test + fun sessionChangeDuringCopyDiscardsImageWithoutTouchingTheNewChat() = + runTest { + val result = image("stale.png") + val gate = CompletableDeferred() + var owner = target + var commits = 0 + val errors = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, _ -> + gate.await() + result + }, + captureTarget = { owner }, + commit = { _, _ -> + commits++ + true + }, + onError = errors::add, + ) + controller.receiveImages(listOf(mockk()), Any()) + runCurrent() + owner = target.copy(generation = 3) + gate.complete(Unit) + coroutineContext.job.children + .toList() + .joinAll() + assertEquals(0, commits) + assertFalse(result.file.exists()) + assertTrue(errors.isEmpty()) + } + + @Test + fun disposalCancelsCopyAndDeletesOnlyUncommittedFiles() = + runTest { + val first = image("cancel.png") + var count = 0 + var commits = 0 + val errors = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, _ -> if (count++ == 0) first else awaitCancellation() }, + captureTarget = { target }, + commit = { _, _ -> + commits++ + true + }, + onError = errors::add, + ) + controller.receiveImages(listOf(mockk(), mockk()), Any()) + runCurrent() + controller.close() + coroutineContext.job.children + .toList() + .joinAll() + assertFalse(first.file.exists()) + assertEquals(0, commits) + assertTrue(errors.isEmpty()) + assertFalse(controller.receiveImages(listOf(mockk()), Any())) + } + + @Test + fun unreadySessionNeverOpensTheProvider() = + runTest { + var reads = 0 + val errors = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, _ -> + reads++ + null + }, + captureTarget = { null }, + commit = { _, _ -> error("Must not commit") }, + onError = errors::add, + ) + assertTrue(controller.receiveImages(listOf(mockk()), Any())) + assertEquals(0, reads) + assertEquals(listOf(R.string.chat_session_not_ready), errors) + assertFalse(controller.isReceiving) + } + + @Test + fun secondPasteWhileImportingDoesNotStartAnotherBatch() = + runTest { + val result = image("busy.png") + val gate = CompletableDeferred() + var reads = 0 + val errors = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, _ -> + reads++ + gate.await() + result + }, + captureTarget = { target }, + commit = { _, _ -> true }, + onError = errors::add, + ) + controller.receiveImages(listOf(mockk()), Any()) + controller.receiveImages(listOf(mockk()), Any()) + gate.complete(Unit) + coroutineContext.job.children + .toList() + .joinAll() + assertEquals(1, reads) + assertEquals(listOf(R.string.chat_image_paste_busy), errors) + } + + @Test + fun ninthImageRejectsBeforeOpeningAnyProvider() = + runTest { + var reads = 0 + val errors = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, _ -> + reads++ + null + }, + captureTarget = { target }, + commit = { _, _ -> error("Must not commit") }, + onError = errors::add, + ) + assertTrue(controller.receiveImages(List(9) { mockk() }, Any())) + runCurrent() + assertEquals(0, reads) + assertEquals(listOf(R.string.chat_image_paste_batch_limit), errors) + assertFalse(controller.isReceiving) + } + + @Test + fun exactBatchLimitKeepsQueuedFilesAndPassesRemainingBudget() = + runTest { + val first = image("first-large.png") + val second = image("second-large.png") + listOf(first, second).forEach { + RandomAccessFile(it.file, "rw").use { file -> + file.setLength(10L * 1024 * 1024) + } + } + val caps = mutableListOf() + val committed = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, cap -> + caps += cap + if (caps.size == 1) first else second + }, + captureTarget = { target }, + commit = { _, attachments -> + committed.addAll(attachments) + true + }, + onError = { error("Unexpected error: $it") }, + ) + controller.receiveImages(List(2) { mockk() }, Any()) + runCurrent() + assertEquals(listOf(10L * 1024 * 1024, 10L * 1024 * 1024), caps) + assertEquals(listOf(first.attachment, second.attachment), committed) + assertTrue(first.file.exists() && second.file.exists()) + } + + @Test + fun exhaustedBudgetRejectsNextWithoutIoAndRollsBack() = + runTest { + val first = image("full-a.png") + val second = image("full-b.png") + listOf(first, second).forEach { + RandomAccessFile(it.file, "rw").use { file -> + file.setLength(10L * 1024 * 1024) + } + } + var reads = 0 + var commits = 0 + val errors = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, _ -> if (++reads == 1) first else second }, + captureTarget = { target }, + commit = { _, _ -> + commits++ + true + }, + onError = errors::add, + ) + controller.receiveImages(List(3) { mockk() }, Any()) + coroutineContext.job.children + .toList() + .joinAll() + assertEquals(2, reads) + assertEquals(0, commits) + assertFalse(first.file.exists() || second.file.exists()) + assertEquals(listOf(R.string.chat_image_paste_batch_limit), errors) + } + + @Test + fun remainingBudgetOverflowReportsBatchLimitAndCleansUp() = + runTest { + val first = image("partial-a.png") + val second = image("partial-b.png") + RandomAccessFile(first.file, "rw").use { it.setLength(10L * 1024 * 1024) } + RandomAccessFile(second.file, "rw").use { it.setLength(2L * 1024 * 1024) } + val caps = mutableListOf() + val errors = mutableListOf() + val controller = + ChatImagePasteController( + this, + importImage = { _, cap -> + caps += cap + when (caps.size) { + 1 -> first + 2 -> second + else -> throw PastedImageTooLargeException() + } + }, + captureTarget = { target }, + commit = { _, _ -> error("Must not commit") }, + onError = errors::add, + ) + controller.receiveImages(List(3) { mockk() }, Any()) + coroutineContext.job.children + .toList() + .joinAll() + assertEquals(listOf(10L * 1024 * 1024, 10L * 1024 * 1024, 8L * 1024 * 1024), caps) + assertFalse(first.file.exists() || second.file.exists()) + assertEquals(listOf(R.string.chat_image_paste_batch_limit), errors) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiStateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiStateTest.kt index ad031b43..a2ccd174 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiStateTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiStateTest.kt @@ -10,6 +10,9 @@ import com.m57.hermescontrol.ui.chat.MessageRole import com.m57.hermescontrol.ui.chat.PendingSend import com.m57.hermescontrol.ui.chat.PendingSendState import com.m57.hermescontrol.ui.chat.StreamingState +import com.m57.hermescontrol.ui.chat.VaultCodePromptUi +import com.m57.hermescontrol.ui.chat.VaultSaveLoginPromptUi +import com.m57.hermescontrol.ui.chat.VaultUnlockPromptUi import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -22,6 +25,54 @@ class TranscriptUiStateTest { private val historical = ChatMessage(id = "historical", role = MessageRole.USER, content = "older") private val clarify = ClarifyUi(text = "choose") + @Test + fun emptyStateDoesNotHideOtherRenderableTailContent() { + val empty = TranscriptUiState.resolve(ChatUiState(), ChatTimelineState(), StreamingState(), null, null) + assertTrue(shouldShowChatEmptyState(empty, hasReplyError = false)) + val visibleStates = + listOf( + empty.copy(vaultUnlockPrompt = VaultUnlockPromptUi("unlock", "session")), + empty.copy(vaultSaveLoginPrompt = VaultSaveLoginPromptUi("save", "session")), + empty.copy(vaultCodePrompt = VaultCodePromptUi("code", "session")), + empty.copy(isCompressing = true), + empty.copy(compressionStatus = "Compressing"), + empty.copy(streamingState = StreamingState(streamingMessage = live)), + empty.copy(isAgentTyping = true), + empty.copy(isLoading = true), + empty.copy(messages = listOf(live)), + ) + visibleStates.forEach { assertFalse(shouldShowChatEmptyState(it, hasReplyError = false)) } + assertFalse(shouldShowChatEmptyState(empty, hasReplyError = true)) + } + + @Test + fun `clarify on empty live transcript uses list rather than empty state`() { + val transcript = + TranscriptUiState.resolve( + ChatUiState(currentSessionId = "session", clarifyRequest = clarify), + ChatTimelineState(), + StreamingState(), + null, + null, + ) + assertSame(clarify, transcript.clarifyRequest) + assertFalse(shouldShowChatEmptyState(transcript, hasReplyError = false)) + } + + @Test + fun `empty historical snapshot remains empty without displaying live clarify`() { + val transcript = + TranscriptUiState.resolve( + ChatUiState(currentSessionId = "session", clarifyRequest = clarify), + ChatTimelineState(historyMessages = emptyList()), + StreamingState(), + null, + null, + ) + assertNull(transcript.clarifyRequest) + assertTrue(shouldShowChatEmptyState(transcript, hasReplyError = false)) + } + @Test fun acceptedLivePromptKeepsItsTextAndImageUntilHistoryConfirmsIt() { val image = Attachment("file:///private/photo", "photo.png", "image/png", 3)