diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ReplyErrorCardTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ReplyErrorCardTest.kt new file mode 100644 index 00000000..dfe453f5 --- /dev/null +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/components/ReplyErrorCardTest.kt @@ -0,0 +1,139 @@ +package com.m57.hermescontrol.ui.chat.components + +import androidx.activity.ComponentActivity +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.width +import androidx.compose.ui.Modifier +import androidx.compose.ui.semantics.SemanticsProperties +import androidx.compose.ui.test.SemanticsMatcher +import androidx.compose.ui.test.assert +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsEnabled +import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.unit.dp +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.m57.hermescontrol.R +import com.m57.hermescontrol.theme.HermesControlTheme +import com.m57.hermescontrol.ui.chat.ReplyFailure +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ReplyErrorCardTest { + @get:Rule + val compose = createAndroidComposeRule() + + @Test + fun detailsStartCollapsedAndCopyIsScrubbedWithoutAutomaticSharing() { + var copied = "" + var shared = false + compose.setContent { + HermesControlTheme { + Box(Modifier.width(320.dp)) { + ReplyErrorCard( + failure = ReplyFailure("HTTP 429\nAuthorization: Bearer private-key"), + onDismiss = {}, + onOpenLogs = {}, + onCopy = { copied = it }, + onShare = { shared = true }, + ) + } + } + } + compose.onNodeWithText("HTTP 429", substring = true).assertDoesNotExist() + + compose.onNodeWithText(compose.activity.getString(R.string.chat_reply_failed_details)).performClick() + compose.onNodeWithText("HTTP 429", substring = true).assertIsDisplayed() + compose.onNodeWithText("private-key", substring = true).assertDoesNotExist() + compose.onNodeWithText(compose.activity.getString(R.string.chat_reply_failed_copy)).performClick() + compose.runOnIdle { + assertEquals("HTTP 429\nAuthorization: [REDACTED]", copied) + assertFalse(shared) + } + } + + @Test + fun sharingRequiresExpandedReviewAndUsesExactSanitizedPayload() { + var shared = "" + var logsOpened = 0 + var dismissed = 0 + compose.setContent { + HermesControlTheme { + ReplyErrorCard( + failure = ReplyFailure("Provider failed\nAuthorization: Bearer private-token"), + onDismiss = { dismissed++ }, + onOpenLogs = { logsOpened++ }, + onCopy = {}, + onShare = { shared = it }, + ) + } + } + compose.runOnIdle { + assertEquals("", shared) + assertEquals(0, logsOpened) + assertEquals(0, dismissed) + } + val detailsLabel = compose.activity.getString(R.string.chat_reply_failed_details) + val collapsedLabel = compose.activity.getString(R.string.chat_reply_failed_details_collapsed) + val expandedLabel = compose.activity.getString(R.string.chat_reply_failed_details_expanded) + val shareLabel = compose.activity.getString(R.string.chat_reply_failed_share) + + compose + .onNodeWithText(detailsLabel) + .assert(SemanticsMatcher.expectValue(SemanticsProperties.StateDescription, collapsedLabel)) + compose.onNodeWithText(shareLabel).assertIsNotEnabled().performClick() + compose.runOnIdle { assertEquals("", shared) } + + compose.onNodeWithText(detailsLabel).performClick() + compose + .onNodeWithText(detailsLabel) + .assert(SemanticsMatcher.expectValue(SemanticsProperties.StateDescription, expandedLabel)) + compose.onNodeWithText(shareLabel).assertIsEnabled().performClick() + compose.onNodeWithText(compose.activity.getString(R.string.chat_reply_failed_logs)).performClick() + compose + .onNodeWithContentDescription( + compose.activity.getString(R.string.chat_reply_failed_dismiss), + ).performClick() + compose.runOnIdle { + assertEquals("Provider failed\nAuthorization: [REDACTED]", shared) + assertEquals(1, logsOpened) + assertEquals(1, dismissed) + } + } + + @Test + fun expandedReviewResetsForANewFailure() { + val failure = androidx.compose.runtime.mutableStateOf(ReplyFailure("First failure")) + compose.setContent { + HermesControlTheme { + ReplyErrorCard( + failure = failure.value, + onDismiss = {}, + onOpenLogs = {}, + onCopy = {}, + onShare = {}, + ) + } + } + + val detailsLabel = compose.activity.getString(R.string.chat_reply_failed_details) + val collapsedLabel = compose.activity.getString(R.string.chat_reply_failed_details_collapsed) + val shareLabel = compose.activity.getString(R.string.chat_reply_failed_share) + compose.onNodeWithText(detailsLabel).performClick() + compose.onNodeWithText(shareLabel).assertIsEnabled() + + compose.runOnIdle { failure.value = ReplyFailure("Second failure") } + + compose + .onNodeWithText(detailsLabel) + .assert(SemanticsMatcher.expectValue(SemanticsProperties.StateDescription, collapsedLabel)) + compose.onNodeWithText(shareLabel).assertIsNotEnabled() + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/notification/ChatNotificationService.kt b/app/src/main/java/com/m57/hermescontrol/notification/ChatNotificationService.kt index 831aa535..48342326 100644 --- a/app/src/main/java/com/m57/hermescontrol/notification/ChatNotificationService.kt +++ b/app/src/main/java/com/m57/hermescontrol/notification/ChatNotificationService.kt @@ -17,6 +17,7 @@ import com.m57.hermescontrol.data.remote.NetworkMonitor 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.ui.chat.replyFailureFromPayload import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -27,6 +28,46 @@ import kotlinx.coroutines.flow.collect import kotlinx.coroutines.launch import java.util.concurrent.atomic.AtomicBoolean +internal data class MessageCompleteNotificationPlan( + val text: String, + val sessionId: String?, + val isReplyMessage: Boolean, + val completionId: String?, + val correlationText: String?, + val allowInlineReply: Boolean, +) + +internal fun messageCompleteNotificationPlan( + event: WsEvent.MessageComplete, + targetSessionId: String?, + newMessageText: String, + failureText: String, +): MessageCompleteNotificationPlan { + if (replyFailureFromPayload(event.rawPayload, fallback = "") != null) { + return MessageCompleteNotificationPlan( + text = failureText, + sessionId = targetSessionId, + isReplyMessage = false, + completionId = null, + correlationText = null, + allowInlineReply = false, + ) + } + + return MessageCompleteNotificationPlan( + text = + event.text + .take(100) + .replace("\n", " ") + .ifBlank { newMessageText }, + sessionId = targetSessionId, + isReplyMessage = true, + completionId = event.completionId, + correlationText = event.text, + allowInlineReply = true, + ) +} + /** * Foreground service that keeps the WebSocket connection alive while the app * is backgrounded and posts a notification when a new assistant reply @@ -111,19 +152,22 @@ class ChatNotificationService : Service() { if (!isAppInForeground.get()) { when (event) { is WsEvent.MessageComplete -> { - val preview = - event.text - .take(100) - .replace("\n", " ") - .ifBlank { getString(R.string.notif_new_message) } val targetSessionId = event.storedSessionId ?: ActiveSessionHolder.resolveStoredSessionId(event.sessionId) + val plan = + messageCompleteNotificationPlan( + event = event, + targetSessionId = targetSessionId, + newMessageText = getString(R.string.notif_new_message), + failureText = getString(R.string.chat_reply_failed_title), + ) showReplyNotification( - text = preview, - sessionId = targetSessionId, - isReplyMessage = true, - completionId = event.completionId, + text = plan.text, + sessionId = plan.sessionId, + isReplyMessage = plan.isReplyMessage, + completionId = plan.completionId, + allowInlineReply = plan.allowInlineReply, // The durable REST row for this turn, when the // boundary armed before the prompt was submitted // still lets us name it unambiguously. Null is a @@ -132,7 +176,9 @@ class ChatNotificationService : Service() { // is strictly better than dismissing the wrong // duplicate reply. serverMessageId = - coalesceTurnRow(targetSessionId, event.text), + plan.correlationText?.let { + coalesceTurnRow(plan.sessionId, it) + }, ) // The wait is over — retire the foreground // service. The reply notification above @@ -216,6 +262,7 @@ class ChatNotificationService : Service() { isReplyMessage: Boolean = false, completionId: String? = null, serverMessageId: Int? = null, + allowInlineReply: Boolean = true, ) { val builder = NotificationCompat @@ -262,7 +309,7 @@ class ChatNotificationService : Service() { ) } - if (!sessionId.isNullOrBlank()) { + if (allowInlineReply && !sessionId.isNullOrBlank()) { val replyLabel = getString(R.string.notif_reply_placeholder) val remoteInput = RemoteInput 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 65e30861..78850553 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 @@ -64,9 +64,11 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.platform.LocalClipboardManager import androidx.compose.ui.platform.LocalContext 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 @@ -76,8 +78,10 @@ import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.repeatOnLifecycle import androidx.lifecycle.viewmodel.compose.viewModel +import com.m57.hermescontrol.BuildConfig import com.m57.hermescontrol.ExternalActivityLifecycleGuard import com.m57.hermescontrol.HistoryScreen +import com.m57.hermescontrol.LogsScreen import com.m57.hermescontrol.NavigationController import com.m57.hermescontrol.R import com.m57.hermescontrol.data.model.Attachment @@ -102,6 +106,7 @@ import com.m57.hermescontrol.ui.chat.components.ContextDetailSheet import com.m57.hermescontrol.ui.chat.components.ContextUsageChip import com.m57.hermescontrol.ui.chat.components.ReactionHeartsOverlay import com.m57.hermescontrol.ui.chat.components.ReloginDialog +import com.m57.hermescontrol.ui.chat.components.ReplyErrorCard import com.m57.hermescontrol.ui.chat.components.SearchBarRow import com.m57.hermescontrol.ui.chat.components.SessionIntegrationsSheet import com.m57.hermescontrol.ui.chat.components.SideQuestionSheet @@ -784,6 +789,39 @@ fun ChatScreen( savingAttachmentPath = pendingSavePath ?: state.savingAttachmentPath, openingAttachmentPath = state.openingAttachmentPath, onImageClick = { viewingImage = it }, + replyErrorContent = + state.replyFailure?.takeUnless { timelineState.isHistorical }?.let { failure -> + { + val clipboard = LocalClipboardManager.current + val copiedMessage = stringResource(R.string.chat_reply_failed_copied) + val shareTitle = stringResource(R.string.chat_reply_failed_share) + val shareUnavailable = stringResource(R.string.chat_reply_failed_share_unavailable) + ReplyErrorCard( + failure = failure, + onDismiss = { viewModel.dismissReplyFailure(failure.id) }, + onOpenLogs = { NavigationController.navigateTo(LogsScreen) }, + onCopy = { details -> + clipboard.setText(AnnotatedString(details)) + scrollScope.launch { snackbarHostState.showSnackbar(copiedMessage) } + }, + onShare = { details -> + val report = "Hermes Mobile ${BuildConfig.VERSION_NAME}\n\n$details" + val intent = + Intent(Intent.ACTION_SEND).apply { + type = "text/plain" + putExtra(Intent.EXTRA_TEXT, report) + } + try { + launchExternalActivity { + context.startActivity(Intent.createChooser(intent, shareTitle)) + } + } catch (_: ActivityNotFoundException) { + scrollScope.launch { snackbarHostState.showSnackbar(shareUnavailable) } + } + }, + ) + } + }, ) // Loading overlay 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 f1a32bfe..dfdabd63 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 @@ -137,6 +137,10 @@ data class ChatUiState( /** Standalone streaming message — rendered after the main list. */ val streamingMessage: ChatMessage? = null, val errorMessage: String? = null, + /** Persistent until dismissed/new turn; distinct from one-shot RPC/network snackbars. */ + val replyFailure: ReplyFailure? = null, + /** Identity of the single non-durable failed assistant projection for this session lifecycle. */ + val replyFailureProjection: ReplyFailureProjection? = null, // Background job completion toast (issue #527) — non-blocking snackbar val backgroundCompleteMessage: String? = null, // Attachment feedback — surfaced as a non-blocking snackbar (issue #724) @@ -842,6 +846,23 @@ class ChatViewModel( } private fun handleWsEvent(event: WsEvent) { + // Filter explicitly scoped terminal/start events before buffer resets and reduction. + // Legacy unscoped failures are ambiguous across sessions, so fail closed: gateways + // must include session_id for failed replies to be surfaced by this client. + val turnSessionId = + when (event) { + is WsEvent.MessageStart -> event.sessionId + is WsEvent.MessageComplete -> event.sessionId + is WsEvent.MessageDone -> event.sessionId + else -> null + } + if (turnSessionId != null && !isCurrentSession(turnSessionId)) return + if (event is WsEvent.MessageComplete && event.rawPayload?.get("status") == "error" && + event.sessionId == null + ) { + return + } + // RpcError is reduced before ViewModel request handling. Drop stale // session errors here so the shared reducer cannot clear loading or // surface an error for a newly selected session. @@ -1238,10 +1259,13 @@ class ChatViewModel( // ── Message streaming ──────────────────────────────────────────────── - /** - * Checks if an incoming WS event belongs to the currently active - * session. Returns true if the event should be processed. - */ + fun dismissReplyFailure(id: String) { + _uiState.update { state -> + if (state.replyFailure?.id == id) state.copy(replyFailure = null) else state + } + } + + /** Checks if an incoming WS event belongs to the currently active session. */ private fun isCurrentSession(eventSessionId: String?): Boolean { // If the event has no session ID, process it (legacy compatibility) if (eventSessionId == null) return true @@ -1510,20 +1534,23 @@ class ChatViewModel( WsMethods.SESSION_RESUME -> { val resultMap = result as? Map + // A resume response may carry retained terminal state. Correlate the + // exact request and stored session before binding the runtime id or + // projecting any failure; a late response must not touch the new chat. + val resumeRequest = request ?: return + val sessionId = resumeRequest.sessionId ?: return + if (sessionId != _uiState.value.currentSessionId) return + val resumedStoredId = (resultMap?.get("resumed") as? String)?.takeIf { it.isNotBlank() } + if (resumedStoredId != null && resumedStoredId != sessionId) return val runtimeId = (resultMap?.get("session_id") as? String)?.takeIf { it.isNotBlank() } if (runtimeId == null) { - val sessionId = request?.sessionId ?: _uiState.value.currentSessionId ?: return - handleResumeFailure(sessionId, sessionGeneration, "Invalid session resume response") + handleResumeFailure(sessionId, resumeRequest.generation, "Invalid session resume response") return } runtimeSessionId = runtimeId connectionOperationDelegate.bindSession(runtimeId) // Resume succeeded — the gateway confirmed the DB row. sessionHasServerPresence = true - val sessionId = - request?.sessionId - ?: (resultMap["resumed"] as? String) - ?: _uiState.value.currentSessionId // Parse session info from backend — model, provider, reasoning_effort val infoMap = resultMap["info"] as? Map @@ -1592,7 +1619,8 @@ class ChatViewModel( ActiveSessionHolder.set(runtimeSessionId ?: sessionId, sessionId) addSystemMessage("Session resumed") fetchContextUsage() - val generation = request?.generation ?: sessionGeneration + projectRetainedReplyFailure(resultMap["inflight"] as? Map, runtimeId) + val generation = resumeRequest.generation resumedGeneration = generation finishResumeWhenHydrated(generation) subagentsDelegate.hydrateSubagents(runtimeSessionId ?: sessionId) @@ -1663,6 +1691,25 @@ class ChatViewModel( } } + /** + * Replays a gateway-retained failed turn through the same reducer path as a + * live terminal `message.complete(status=error)`. The assistant field is + * the partial display projection; user/transcript fields are never exported + * as diagnostics, and reducer effects intentionally do not persist it. + */ + private fun projectRetainedReplyFailure( + inflight: Map?, + runtimeId: String, + ) { + if (inflight?.get("status") != "error") return + val result = + ChatWsEventReducer.reduceRetainedReplyFailure(_uiState.value, inflight, runtimeId) + _uiState.value = result.state + _streamingState.value = result.streamingState + dispatchReducerEffects(result.effects) + streamingController.resetStreaming() + } + @Suppress("UNCHECKED_CAST") private fun handleDispatchResult(result: Any?) { val map = result as? Map<*, *> ?: return @@ -3546,6 +3593,8 @@ class ChatViewModel( hasOlderMessages = false, streamingMessage = null, errorMessage = null, + replyFailure = null, + replyFailureProjection = null, openError = null, clarifyRequest = null, sudoPrompt = null, 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 7260e860..28d248be 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 @@ -69,6 +69,40 @@ private fun stripSealedOrphanPrefix( * Android or ViewModel classes. Easy to unit test. */ object ChatWsEventReducer { + /** + * Projects retained terminal state without consulting any live streaming buffers. + * Only `inflight.assistant` may become assistant prose; stale UI text, reasoning, + * and message ids are deliberately excluded. + */ + fun reduceRetainedReplyFailure( + state: ChatUiState, + inflight: Map, + currentSessionId: String, + ): ReducerResult { + if (inflight["status"] != "error") return ReducerResult(state, StreamingState()) + val assistant = (inflight["assistant"] as? String).orEmpty() + val payload = inflight.toMutableMap().apply { put("partial", assistant.isNotBlank()) } + val cleanState = + state.copy( + isAgentTyping = false, + isThinking = false, + thinkingText = "", + streamingMessage = null, + ) + return reduce( + cleanState, + StreamingState(), + WsEvent.MessageComplete( + text = assistant, + sessionId = currentSessionId, + // Retained inflight has no completion identity; suppress the event's random default. + completionId = "", + rawPayload = payload, + ), + currentSessionId, + ) + } + fun reduce( state: ChatUiState, streamingState: StreamingState, @@ -239,6 +273,18 @@ object ChatWsEventReducer { streamingState: StreamingState, event: WsEvent.MessageStart, ): ReducerResult { + val previousFailureMessageId = state.replyFailureProjection?.messageId + val turnState = + state.copy( + messages = + if (previousFailureMessageId == null) { + state.messages + } else { + state.messages.filterNot { it.id == previousFailureMessageId } + }, + replyFailure = null, + replyFailureProjection = null, + ) val msg = ChatMessage( role = MessageRole.ASSISTANT, @@ -252,10 +298,10 @@ object ChatWsEventReducer { val effects = mutableListOf() val cleanedSubagents = - if (state.subagentIndicators.all { it.isComplete || it.isFailed }) { + if (turnState.subagentIndicators.all { it.isComplete || it.isFailed }) { emptyList() } else { - state.subagentIndicators + turnState.subagentIndicators } // Build new state: finalize any orphan streaming message, then set the new one @@ -270,13 +316,13 @@ object ChatWsEventReducer { tps = null, ) orphan = finalized - state.copy( - messages = state.messages.upsertById(finalized), + turnState.copy( + messages = turnState.messages.upsertById(finalized), isAgentTyping = true, subagentIndicators = cleanedSubagents, ) } else { - state.copy( + turnState.copy( isAgentTyping = true, subagentIndicators = cleanedSubagents, ) @@ -406,6 +452,8 @@ object ChatWsEventReducer { streamingState: StreamingState, event: WsEvent.MessageComplete, ): ReducerResult { + val failure = replyFailureFromPayload(event.rawPayload, event.text) + if (failure != null) return onReplyFailure(state, streamingState, event, failure) val finalSnapshot = event.rawPayload?.let(::parseUsageSnapshot) val usageState = finalSnapshot?.let { applyUsageSnapshot(state, it) } ?: state val turnUsage = finalSnapshot?.deltaFrom(streamingState.turnUsageBaseline) @@ -520,6 +568,86 @@ object ChatWsEventReducer { ) } + // Keep terminal diagnostics out of normal assistant messages and persistence. + private fun onReplyFailure( + state: ChatUiState, + streamingState: StreamingState, + event: WsEvent.MessageComplete, + failure: ReplyFailure, + ): ReducerResult { + val usageState = event.rawPayload?.let(::parseUsageSnapshot)?.let { applyUsageSnapshot(state, it) } ?: state + val streaming = streamingState.streamingMessage + val text = + if (event.rawPayload?.get("partial") == true) { + event.text + .takeIf { it.isNotBlank() } + ?.let { stripSealedOrphanPrefix(it, state.messages, streamingState.sealedOrphanIds) } + ?: streaming?.content.orEmpty() + } else { + streaming?.content.orEmpty() + } + val reasoning = streamingState.reasoningText.ifBlank { streaming?.reasoningText.orEmpty() } + val existingProjection = state.replyFailureProjection + val completionId = event.completionId?.takeIf { it.isNotBlank() } ?: existingProjection?.completionId + val sessionIdentity = state.currentSessionId ?: event.sessionId.orEmpty() + val failureId = + existingProjection?.failureId + ?: event.completionId?.takeIf { it.isNotBlank() }?.let { "reply-failure-$sessionIdentity-$it" } + ?: failure.id + val projectionMessageId = + existingProjection?.messageId + ?: streaming?.id + ?: event.completionId?.takeIf { it.isNotBlank() }?.let { "reply-failure-message-$it" } + ?: if (text.isNotBlank() || reasoning.isNotBlank()) "reply-failure-message-$failureId" else null + val projection = ReplyFailureProjection(projectionMessageId, failureId, completionId) + val existingProjectedMessage = + projectionMessageId?.let { id -> usageState.messages.firstOrNull { it.id == id } } + val partial = + if (text.isNotBlank() || reasoning.isNotBlank()) { + val base = + streaming + ?: existingProjectedMessage + ?: ChatMessage( + id = checkNotNull(projectionMessageId), + role = MessageRole.ASSISTANT, + content = "", + ) + base.copy( + id = checkNotNull(projectionMessageId), + content = text, + reasoningText = reasoning, + isStreaming = false, + finishTimestamp = System.currentTimeMillis(), + completionId = completionId, + ) + } else { + null + } + val messagesWithoutOldProjection = + existingProjection?.messageId?.let { oldId -> usageState.messages.filterNot { it.id == oldId } } + ?: usageState.messages + val projectedMessages = + partial?.let { messagesWithoutOldProjection.upsertById(it) } ?: messagesWithoutOldProjection + // A failed partial is a transient projection of gateway `inflight`, not a + // successful transcript row. Persisting it would erase failure provenance on + // hydration and make a later session reopen look healthy. + val effects = listOf(ReducerEffect.RefreshSessions, ReducerEffect.RefreshContextUsage) + return ReducerResult( + state = + usageState.copy( + messages = projectedMessages, + replyFailure = failure.copy(id = failureId), + replyFailureProjection = projection, + isAgentTyping = false, + isThinking = false, + streamingMessage = null, + clarifyRequest = null, + ), + streamingState = StreamingState(), + effects = effects, + ) + } + // ── MessageDone ─────────────────────────────────────────────────── private fun onMessageDone( diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ReplyFailure.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ReplyFailure.kt new file mode 100644 index 00000000..cca7527c --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ReplyFailure.kt @@ -0,0 +1,59 @@ +package com.m57.hermescontrol.ui.chat + +import java.util.UUID + +/** Session-scoped presentation, never a send receipt or a persisted transcript row. */ +data class ReplyFailure( + val details: String, + val id: String = UUID.randomUUID().toString(), +) + +/** Stable identity for the one transient failed-reply projection in a session lifecycle. */ +data class ReplyFailureProjection( + val messageId: String?, + val failureId: String, + val completionId: String? = null, +) + +/** Only a terminal turn failure is a failed reply; RPC/transport errors are separate. */ +internal fun replyFailureFromPayload( + payload: Map?, + fallback: String, +): ReplyFailure? { + if (payload?.get("status") != "error") return null + val surface = payload["error_surface"] as? Map<*, *> + val details = + buildList { + for (key in listOf("layer", "code", "provider", "model")) { + (surface?.get(key) as? String)?.takeIf { it.isNotBlank() }?.let { add("$key: $it") } + } + val error = (payload["error"] as? String)?.takeIf { it.isNotBlank() } + add(error ?: if (payload["partial"] == true) "Unknown gateway error" else fallback) + }.joinToString("\n") + return ReplyFailure(sanitizeReplyErrorDetails(details)) +} + +/** Best-effort scrubbing before display/copy/share; never includes a transcript or request payload. */ +internal fun sanitizeReplyErrorDetails(raw: String?): String { + if (raw.isNullOrBlank()) return "Unknown gateway error" + var safe = raw.take(8_192) + safe = PRIVATE_KEY.replace(safe, "[REDACTED]") + safe = HEADER.replace(safe) { "${it.groupValues[1]}[REDACTED]" } + safe = URL.replace(safe, "[REDACTED URL]") + safe = SECRET_ASSIGNMENT.replace(safe) { "${it.groupValues[1]}[REDACTED]" } + safe = BEARER.replace(safe, "Bearer [REDACTED]") + safe = PROVIDER_KEY.replace(safe, "[REDACTED]") + return safe.take(8_192) +} + +private val PRIVATE_KEY = Regex("-----BEGIN [^-]*PRIVATE KEY-----[\\s\\S]*?(?:-----END [^-]*PRIVATE KEY-----|$)") +private val HEADER = Regex("(?im)^([ \\t]*(?:authorization|proxy-authorization|set-cookie|cookie)\\s*:\\s*)[^\\r\\n]*") +private val URL = Regex("(?i)(?:https?|wss?)://[^\\s<>\\\"']+") +private val SECRET_ASSIGNMENT = + Regex( + "(?i)((? Unit, + onOpenLogs: () -> Unit, + onCopy: (String) -> Unit, + onShare: (String) -> Unit, +) { + var expanded by rememberSaveable(failure.id) { mutableStateOf(false) } + + val details = remember(failure.details) { sanitizeReplyErrorDetails(failure.details) } + val detailsStateDescription = + stringResource( + if (expanded) { + R.string.chat_reply_failed_details_expanded + } else { + R.string.chat_reply_failed_details_collapsed + }, + ) + val colors = LocalHermesStatusColors.current + Surface( + modifier = Modifier.fillMaxWidth().padding(12.dp).testTag("reply_error_card"), + shape = MaterialTheme.shapes.medium, + color = colors.errorContainer, + contentColor = colors.onErrorContainer, + border = BorderStroke(1.dp, colors.error), + ) { + Column(Modifier.padding(12.dp), verticalArrangement = Arrangement.spacedBy(6.dp)) { + Row(verticalAlignment = Alignment.CenterVertically) { + Icon(Icons.Default.ErrorOutline, contentDescription = null) + Text( + stringResource(R.string.chat_reply_failed_title), + modifier = + Modifier + .weight( + 1f, + ).padding(start = 8.dp) + .semantics { liveRegion = LiveRegionMode.Polite }, + style = MaterialTheme.typography.titleSmall, + ) + IconButton(onClick = onDismiss) { + Icon(Icons.Default.Close, contentDescription = stringResource(R.string.chat_reply_failed_dismiss)) + } + } + Text(stringResource(R.string.chat_reply_failed_description), style = MaterialTheme.typography.bodyMedium) + TextButton( + onClick = { expanded = !expanded }, + modifier = Modifier.semantics { stateDescription = detailsStateDescription }, + ) { + Icon(if (expanded) Icons.Default.ExpandLess else Icons.Default.ExpandMore, contentDescription = null) + Text(stringResource(R.string.chat_reply_failed_details)) + } + if (expanded) { + SelectionContainer { + Text( + details, + modifier = Modifier.heightIn(max = 200.dp).verticalScroll(rememberScrollState()), + style = MaterialTheme.typography.bodySmall, + fontFamily = FontFamily.Monospace, + ) + } + } + Text(stringResource(R.string.chat_reply_failed_share_note), style = MaterialTheme.typography.bodySmall) + FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + OutlinedButton(onClick = onOpenLogs) { Text(stringResource(R.string.chat_reply_failed_logs)) } + OutlinedButton(onClick = { onCopy(details) }) { Text(stringResource(R.string.chat_reply_failed_copy)) } + OutlinedButton( + onClick = { onShare(details) }, + enabled = expanded, + ) { Text(stringResource(R.string.chat_reply_failed_share)) } + } + } + } +} 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 33d24c21..431b0195 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 @@ -104,8 +104,9 @@ fun FullBleedChatList( hasOlderMessages: Boolean = false, pagingSessionId: String? = null, onLoadOlder: () -> Unit = viewModel::loadOlderMessages, + replyErrorContent: (@Composable () -> Unit)? = null, ) { - if (messages.isEmpty() && !isLoading && !isAgentTyping) { + if (messages.isEmpty() && !isLoading && !isAgentTyping && replyErrorContent == null) { Box( modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center, @@ -145,6 +146,7 @@ fun FullBleedChatList( // A single ordered definition supplies both emitted tail rows and anchor keys. val tailItems = buildMap Unit> { + replyErrorContent?.let { put("reply_error", it) } agentStatus?.let { status -> put("agent_status") { AgentStatusIndicator(status = status) diff --git a/app/src/main/res/values/reply_errors.xml b/app/src/main/res/values/reply_errors.xml new file mode 100644 index 00000000..9c3b9ced --- /dev/null +++ b/app/src/main/res/values/reply_errors.xml @@ -0,0 +1,16 @@ + + + Hermes couldn\'t finish this reply + Something went wrong while Hermes was replying. Open the details or logs to understand what happened. + Details + Collapsed + Expanded + Copy error details + Open logs + Share diagnostics + Review details before sharing. Diagnostics include this error and the app version, not your conversation or attachments. + Dismiss error + + Error details copied + No app available to share diagnostics + diff --git a/app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt b/app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt index 4d4fca1c..71e116e7 100644 --- a/app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt @@ -1,9 +1,13 @@ package com.m57.hermescontrol.notification import android.content.Context +import com.m57.hermescontrol.data.ws.WsEvent import io.mockk.mockk import io.mockk.verify import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test /** @@ -104,4 +108,59 @@ class ChatNotificationServiceTest { atomicBoolean.get(), ) } + + @Test + fun `terminal reply error uses generic failure notification without success affordances`() { + val rawDiagnostic = "Authorization: Bearer secret-token" + val plan = + messageCompleteNotificationPlan( + event = + WsEvent.MessageComplete( + text = rawDiagnostic, + sessionId = "runtime-1", + storedSessionId = "stored-1", + completionId = "completion-1", + rawPayload = + mapOf( + "status" to "error", + "error" to rawDiagnostic, + ), + ), + targetSessionId = "stored-1", + newMessageText = "New message", + failureText = "Hermes couldn't finish this reply", + ) + + assertEquals("Hermes couldn't finish this reply", plan.text) + assertFalse(plan.text.contains(rawDiagnostic)) + assertEquals("stored-1", plan.sessionId) + assertFalse(plan.isReplyMessage) + assertFalse(plan.allowInlineReply) + assertNull(plan.completionId) + assertNull(plan.correlationText) + } + + @Test + fun `successful completion keeps reply correlation and inline reply`() { + val plan = + messageCompleteNotificationPlan( + event = + WsEvent.MessageComplete( + text = "Answer\ncontinued", + sessionId = "runtime-1", + completionId = "completion-1", + rawPayload = mapOf("status" to "ok"), + ), + targetSessionId = "stored-1", + newMessageText = "New message", + failureText = "Hermes couldn't finish this reply", + ) + + assertEquals("Answer continued", plan.text) + assertEquals("stored-1", plan.sessionId) + assertTrue(plan.isReplyMessage) + assertTrue(plan.allowInlineReply) + assertEquals("completion-1", plan.completionId) + assertEquals("Answer\ncontinued", plan.correlationText) + } } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatReplyErrorTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatReplyErrorTest.kt new file mode 100644 index 00000000..9ce48c95 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatReplyErrorTest.kt @@ -0,0 +1,315 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.ws.WsEvent +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 + +class ChatReplyErrorTest { + @Test + fun failedCompletionKeepsPartialReplyAndStopsThinking() { + val partial = + ChatMessage(id = "partial", role = MessageRole.ASSISTANT, content = "Partial reply", isStreaming = true) + val state = + ChatUiState( + currentSessionId = "runtime", + isAgentTyping = true, + isThinking = true, + streamingMessage = partial, + ) + val streaming = StreamingState(streamingMessage = partial, isThinking = true) + + val event = + WsEvent.MessageComplete( + text = "Partial reply", + sessionId = "runtime", + completionId = "completion-failed-1", + rawPayload = mapOf("status" to "error", "error" to "Provider failed", "partial" to true), + ) + val result = ChatWsEventReducer.reduce(state, streaming, event, "runtime") + + assertFalse(result.state.isThinking) + assertEquals( + "Partial reply", + result.state.messages + .single { it.id == "partial" } + .content, + ) + assertNotNull(result.state.replyFailure) + assertFalse(result.state.isAgentTyping) + assertNull(result.streamingState.streamingMessage) + assertEquals( + "completion-failed-1", + result.state.messages + .single { it.id == "partial" } + .completionId, + ) + assertTrue(result.effects.none { it is ReducerEffect.PersistMessage }) + } + + @Test + fun terminalErrorIsNeverPersistedAsAssistantProse() { + val event = + WsEvent.MessageComplete( + text = "Provider refused API_KEY=private-value", + sessionId = "runtime", + rawPayload = mapOf("status" to "error"), + ) + val result = ChatWsEventReducer.reduce(ChatUiState(), StreamingState(), event, "runtime") + assertNotNull(result.state.replyFailure) + assertFalse( + result.state.replyFailure!! + .details + .contains("private-value"), + ) + assertTrue(result.state.messages.isEmpty()) + assertTrue(result.effects.none { it is ReducerEffect.PersistMessage }) + } + + @Test + fun errorWithoutPartialFlagStillKeepsAlreadyStreamedProse() { + val streaming = + StreamingState(streamingMessage = ChatMessage(role = MessageRole.ASSISTANT, content = "Visible")) + val event = WsEvent.MessageComplete("Error!", null, rawPayload = mapOf("status" to "error")) + val result = ChatWsEventReducer.reduce(ChatUiState(), streaming, event) + assertEquals( + "Visible", + result.state.messages + .single() + .content, + ) + assertNotNull(result.state.replyFailure) + } + + @Test + fun otherSessionCannotShowFailureOrChangeStreaming() { + val state = ChatUiState(isAgentTyping = true) + val streaming = StreamingState(isThinking = true) + val event = WsEvent.MessageComplete("Error", "other", rawPayload = mapOf("status" to "error")) + val result = ChatWsEventReducer.reduce(state, streaming, event, "current") + assertEquals(state, result.state) + assertEquals(streaming, result.streamingState) + assertTrue(result.effects.isEmpty()) + } + + @Test + fun missingPartialTextPreservesAlreadyVisibleOutput() { + for (text in listOf("", " ")) { + val streaming = + StreamingState( + streamingMessage = + ChatMessage( + id = "visible", + role = MessageRole.ASSISTANT, + content = "Visible output", + ), + ) + val event = + WsEvent.MessageComplete( + text, + null, + rawPayload = mapOf("status" to "error", "partial" to true), + ) + val result = ChatWsEventReducer.reduce(ChatUiState(), streaming, event) + assertEquals( + "Visible output", + result.state.messages + .single() + .content, + ) + } + } + + @Test + fun partialConversationIsNeverUsedAsDiagnosticFallback() { + val event = + WsEvent.MessageComplete( + "Private conversational content", + null, + rawPayload = mapOf("status" to "error", "partial" to true), + ) + val result = ChatWsEventReducer.reduce(ChatUiState(), StreamingState(), event) + assertEquals( + "Private conversational content", + result.state.messages + .single() + .content, + ) + assertFalse( + result.state.replyFailure!! + .details + .contains("Private conversational content"), + ) + assertTrue( + result.state.replyFailure!! + .details + .isNotBlank(), + ) + } + + @Test + fun normalReplyAndGatewayErrorAreNotTerminalReplyFailures() { + for (event in listOf(WsEvent.MessageComplete("Success", null), WsEvent.GatewayError("Connection failed"))) { + val result = ChatWsEventReducer.reduce(ChatUiState(), StreamingState(), event) + assertNull(result.state.replyFailure) + } + } + + @Test + fun nextTurnClearsFailureButDoneDoesNot() { + val failure = ReplyFailure("Failed") + val failedPartial = + ChatMessage(id = "failed-partial", role = MessageRole.ASSISTANT, content = "Partial") + val state = + ChatUiState( + messages = listOf(ChatMessage(id = "normal", role = MessageRole.USER, content = "Keep"), failedPartial), + replyFailure = failure, + replyFailureProjection = ReplyFailureProjection("failed-partial", failure.id), + ) + val done = ChatWsEventReducer.reduce(state, StreamingState(), WsEvent.MessageDone(null)) + assertEquals(failure, done.state.replyFailure) + val start = ChatWsEventReducer.reduce(state, StreamingState(), WsEvent.MessageStart(null)) + assertNull(start.state.replyFailure) + assertNull(start.state.replyFailureProjection) + assertEquals(listOf("normal"), start.state.messages.map { it.id }) + } + + @Test + fun retainedFailureWithEmptyAssistantNeverInheritsUnrelatedStream() { + val normal = ChatMessage(id = "normal", role = MessageRole.USER, content = "Keep") + val stale = + ChatMessage( + id = "unrelated-stream", + role = MessageRole.ASSISTANT, + content = "Stale text", + reasoningText = "Stale reasoning", + isStreaming = true, + ) + val state = + ChatUiState( + messages = listOf(normal), + isAgentTyping = true, + isThinking = true, + thinkingText = "Stale thinking", + streamingMessage = stale, + ) + + val result = + ChatWsEventReducer.reduceRetainedReplyFailure( + state = state, + inflight = mapOf("assistant" to "", "status" to "error", "error" to "Provider failed"), + currentSessionId = "runtime", + ) + + assertEquals(listOf(normal), result.state.messages) + assertFalse(result.state.isAgentTyping) + assertFalse(result.state.isThinking) + assertEquals("", result.state.thinkingText) + assertNull(result.state.streamingMessage) + assertEquals(StreamingState(), result.streamingState) + assertNotNull(result.state.replyFailure) + assertNull(result.state.replyFailureProjection?.messageId) + } + + @Test + fun duplicateRetainedFailureKeepsOneStableProjection() { + val normal = ChatMessage(id = "normal", role = MessageRole.USER, content = "Keep") + val inflight = + mapOf("assistant" to "Retained partial", "status" to "error", "error" to "Provider failed") + + val first = + ChatWsEventReducer.reduceRetainedReplyFailure( + ChatUiState(messages = listOf(normal)), + inflight, + "runtime", + ) + val second = ChatWsEventReducer.reduceRetainedReplyFailure(first.state, inflight, "runtime") + + assertEquals(2, second.state.messages.size) + assertEquals(listOf("Keep", "Retained partial"), second.state.messages.map { it.content }) + assertEquals(first.state.replyFailureProjection, second.state.replyFailureProjection) + assertEquals(first.state.replyFailure?.id, second.state.replyFailure?.id) + } + + @Test + fun duplicateLiveFailureKeepsOneStableProjection() { + val normal = ChatMessage(id = "normal", role = MessageRole.USER, content = "Keep") + val partial = + ChatMessage(id = "live-partial", role = MessageRole.ASSISTANT, content = "Partial", isStreaming = true) + val event = + WsEvent.MessageComplete( + text = "Partial", + sessionId = "runtime", + completionId = "completion-1", + rawPayload = mapOf("status" to "error", "error" to "Provider failed", "partial" to true), + ) + + val first = + ChatWsEventReducer.reduce( + ChatUiState(messages = listOf(normal), currentSessionId = "runtime"), + StreamingState(streamingMessage = partial), + event, + "runtime", + ) + val second = ChatWsEventReducer.reduce(first.state, first.streamingState, event, "runtime") + + assertEquals(listOf("normal", "live-partial"), second.state.messages.map { it.id }) + assertEquals(first.state.replyFailureProjection, second.state.replyFailureProjection) + assertEquals(first.state.replyFailure?.id, second.state.replyFailure?.id) + } + + @Test + fun liveFailureThenResumeUpdatesSameProjection() { + val partial = + ChatMessage( + id = "live-partial", + role = MessageRole.ASSISTANT, + content = "Live partial", + isStreaming = true, + ) + val live = + ChatWsEventReducer.reduce( + ChatUiState(currentSessionId = "stored"), + StreamingState(streamingMessage = partial), + WsEvent.MessageComplete( + text = "Live partial", + sessionId = "runtime", + completionId = "completion-1", + rawPayload = mapOf("status" to "error", "error" to "Provider failed", "partial" to true), + ), + "runtime", + ) + + val resumed = + ChatWsEventReducer.reduceRetainedReplyFailure( + live.state, + mapOf("assistant" to "Retained partial", "status" to "error", "error" to "Provider failed"), + "runtime", + ) + + assertEquals(1, resumed.state.messages.size) + assertEquals( + "live-partial", + resumed.state.messages + .single() + .id, + ) + assertEquals( + "Retained partial", + resumed.state.messages + .single() + .content, + ) + assertEquals( + "completion-1", + resumed.state.messages + .single() + .completionId, + ) + assertEquals(live.state.replyFailureProjection, resumed.state.replyFailureProjection) + assertEquals(live.state.replyFailure?.id, resumed.state.replyFailure?.id) + } +} 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 0bdb03a7..8bd9a1c1 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 @@ -460,6 +460,104 @@ class ChatViewModelTest { assertEquals("op-1218", operation?.opId) } + @Test + fun sessionResume_restoresRetainedFailureAsNonDurablePartial() = + runTest { + val viewModel = createViewModel() + advanceUntilIdle() + viewModel.switchSession("stored-session") + advanceUntilIdle() + val resumeRequestId = sentRequestMethods.last { it.first == WsMethods.SESSION_RESUME }.second + + mockEventsFlow.emit( + WsEvent.RpcResult( + resumeRequestId, + mapOf( + "session_id" to "runtime-session", + "resumed" to "stored-session", + "inflight" to + mapOf( + "assistant" to "Retained partial answer", + "user" to "Private user prompt", + "streaming" to false, + "status" to "error", + "error" to "Provider failed", + "recoverable" to true, + "error_surface" to mapOf("provider" to "example", "code" to "rate_limit"), + ), + ), + ), + ) + advanceUntilIdle() + + assertEquals("stored-session", viewModel.uiState.value.currentSessionId) + assertEquals( + "Retained partial answer", + viewModel.uiState.value.messages + .last() + .content, + ) + assertEquals( + "code: rate_limit\nprovider: example\nProvider failed", + viewModel.uiState.value.replyFailure + ?.details, + ) + assertFalse( + viewModel.uiState.value.replyFailure!! + .details + .contains("Private user prompt"), + ) + assertTrue( + fakeRepo.dao + .getMessagesForSession("stored-session") + .none { it.content == "Retained partial answer" }, + ) + } + + @Test + fun staleSessionResumeFailureCannotClearHealthyCurrentStream() = + runTest { + val viewModel = createViewModel() + advanceUntilIdle() + viewModel.switchSession("stored-old") + advanceUntilIdle() + val staleResumeId = sentRequestMethods.last { it.first == WsMethods.SESSION_RESUME }.second + + viewModel.switchSession("stored-current") + advanceUntilIdle() + mockEventsFlow.emit(WsEvent.MessageStart("stored-current")) + mockEventsFlow.emit(WsEvent.MessageToken("Healthy current stream", "stored-current")) + advanceUntilIdle() + + mockEventsFlow.emit( + WsEvent.RpcResult( + staleResumeId, + mapOf( + "session_id" to "runtime-old", + "inflight" to + mapOf( + "assistant" to "Old failed partial", + "status" to "error", + "error" to "Old provider failure", + ), + ), + ), + ) + advanceUntilIdle() + + assertEquals("stored-current", viewModel.uiState.value.currentSessionId) + assertNull(viewModel.uiState.value.replyFailure) + assertEquals( + "Healthy current stream", + viewModel.streamingState.value.streamingMessage + ?.content, + ) + assertTrue( + viewModel.uiState.value.messages + .none { it.content == "Old failed partial" }, + ) + } + private fun connectionOperationSnapshot( sessionId: String, seq: Long = 1L, @@ -1387,6 +1485,96 @@ class ChatViewModelTest { // ── Streaming tests ────────────────────────────────────────────────────── + @Test + fun terminalReplyFailureIsScopedDismissibleAndNeverResends() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + mockEventsFlow.emit(WsEvent.MessageStart(sessionId)) + mockEventsFlow.emit(WsEvent.MessageToken("Partial", sessionId)) + advanceUntilIdle() + mockEventsFlow.emit( + WsEvent.MessageComplete("Other error", "other-session", rawPayload = mapOf("status" to "error")), + ) + advanceUntilIdle() + assertTrue(viewModel.uiState.value.isAgentTyping) + assertNull(viewModel.uiState.value.replyFailure) + assertEquals( + "Partial", + viewModel.streamingState.value.streamingMessage + ?.content, + ) + mockEventsFlow.emit( + WsEvent.MessageComplete("Provider error", sessionId, rawPayload = mapOf("status" to "error")), + ) + advanceUntilIdle() + val failure = viewModel.uiState.value.replyFailure!! + assertFalse(viewModel.uiState.value.isAgentTyping) + assertFalse(viewModel.uiState.value.isThinking) + assertTrue( + viewModel.uiState.value.messages + .any { it.content == "Partial" }, + ) + assertFalse( + viewModel.uiState.value.messages + .any { it.content == "Provider error" }, + ) + viewModel.dismissReplyFailure("stale-card") + advanceUntilIdle() + assertEquals(failure, viewModel.uiState.value.replyFailure) + viewModel.dismissReplyFailure(failure.id) + advanceUntilIdle() + assertNull(viewModel.uiState.value.replyFailure) + verify(exactly = 0) { HermesWsClient.sendMessage(any(), any(), any(), any()) } + } + + @Test + fun unscopedFailureCannotCrossSessionSwitch() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + val failure = WsEvent.MessageComplete("Error", null, rawPayload = mapOf("status" to "error")) + mockEventsFlow.emit(failure) + advanceUntilIdle() + assertNull(viewModel.uiState.value.replyFailure) + mockEventsFlow.emit(WsEvent.MessageStart(sessionId)) + advanceUntilIdle() + viewModel.switchSession("another-session") + advanceUntilIdle() + // A delayed start without identity cannot authorize a failed turn on this chat. + mockEventsFlow.emit(WsEvent.MessageStart(null)) + advanceUntilIdle() + mockEventsFlow.emit(failure) + advanceUntilIdle() + assertNull(viewModel.uiState.value.replyFailure) + assertFalse( + viewModel.uiState.value.messages + .any { it.content == "Error" }, + ) + } + + @Test + fun unscopedFailureAfterForeignStartCannotUsePreviousSessionPin() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + mockEventsFlow.emit(WsEvent.MessageStart(sessionId)) + mockEventsFlow.emit(WsEvent.MessageToken("Current partial", sessionId)) + advanceUntilIdle() + mockEventsFlow.emit(WsEvent.MessageStart("foreign-session")) + mockEventsFlow.emit( + WsEvent.MessageComplete("Foreign diagnostic", null, rawPayload = mapOf("status" to "error")), + ) + advanceUntilIdle() + assertNull(viewModel.uiState.value.replyFailure) + assertEquals( + "Current partial", + viewModel.streamingState.value.streamingMessage + ?.content, + ) + assertFalse( + viewModel.uiState.value.messages + .any { it.content == "Foreign diagnostic" }, + ) + } + @Test fun testMessageStreamingFlow() = runTest { diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ReplyFailureDetailsTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ReplyFailureDetailsTest.kt new file mode 100644 index 00000000..fa646218 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ReplyFailureDetailsTest.kt @@ -0,0 +1,61 @@ +package com.m57.hermescontrol.ui.chat + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ReplyFailureDetailsTest { + @Test(timeout = 2_000) + fun longNonMatchingDiagnosticDoesNotBlockUi() { + assertEquals("x".repeat(8_192), sanitizeReplyErrorDetails("x".repeat(100_000))) + } + + @Test + fun redactsCredentialsAndUrlsBeforeDisplayOrExport() { + val raw = + """ + Provider failed with HTTP 429 + Authorization: Bearer private-bearer + Cookie: sid=private-cookie; csrf=private-csrf + {"api_key":"private-key", "password": "private password", "access_token": "private-access"} + https://user:private-pass@example.test/path?token=private-query + OPENAI_API_KEY=private-env + -----BEGIN PRIVATE KEY----- + private-key-material + -----END PRIVATE KEY----- + """.trimIndent() + val details = sanitizeReplyErrorDetails(raw) + assertTrue(details.contains("HTTP 429")) + assertTrue(details.contains("[REDACTED]")) + assertFalse(details.contains("private-")) + assertFalse(details.contains("private password")) + } + + @Test + fun limitsDetailsAndHandlesMissingMessage() { + assertTrue(sanitizeReplyErrorDetails("x".repeat(100_000)).length <= 8_192) + assertEquals("Unknown gateway error", sanitizeReplyErrorDetails(null)) + assertEquals("Unknown gateway error", sanitizeReplyErrorDetails(" ")) + } + + @Test + fun structuredSurfaceExportsOnlyAllowedMetadataAndSanitizedError() { + val failure = + replyFailureFromPayload( + mapOf( + "status" to "error", + "error" to "HTTP 429", + "error_surface" to + mapOf( + "provider" to "example", + "code" to "rate_limit", + "unknown_field" to "private-information", + "message" to "private-conversation", + ), + ), + "fallback", + ) + assertEquals("code: rate_limit\nprovider: example\nHTTP 429", failure?.details) + } +}